Refactor path handling and clean up todos

Author: elmisback

src/extension.ts

@@ -231,8 +231,7 @@ export function activate(context: vscode.ExtensionContext) {
 		const url = HERBIE_SERVER_ADDRESS
 		// download with curl to home local share odyssey
 		const home = require('os').homedir()
-		// TODO path.join instead of string concat
-		const odysseyDir = home + '/.local/share/odyssey'
+		const odysseyDir = join(home, '.local', 'share', 'odyssey')
 		if (!fs.existsSync(odysseyDir)) {
 			fs.mkdirSync(odysseyDir, { recursive: true })
 		}
@@ -389,8 +388,8 @@ export function activate(context: vscode.ExtensionContext) {
 		const url = FPTAYLOR_SERVER_ADDRESS
 		// download with curl to home local share odyssey
 		const home = require('os').homedir()
-		// TODO path.join instead of string concat
-		const odysseyDir = home + '/.local/share/odyssey'
+		// Use path.join for proper cross-platform path handling
+		const odysseyDir = join(home, '.local', 'share', 'odyssey')
 		if (!fs.existsSync(odysseyDir)) {
 			fs.mkdirSync(odysseyDir, { recursive: true })
 		}
@@ -437,24 +436,6 @@ export function activate(context: vscode.ExtensionContext) {
 					}
 				})
 			}
-
-			// TODO We don't need this code, why is it here?
-			// // show information message
-			// vscode.window.showInformationMessage('FPTaylor installed successfully. Starting server...')
-			// try {
-			// 	// run the command in the VSCode terminal
-			// 	// show the terminal
-			// 	herbieTerminal = getTerminal()
-			// 	herbieTerminal.show()
-
-			// 	herbieTerminal.sendText(fptaylorPath + ' web --quiet')
-			// } catch (err: any) {
-			// 	vscode.window.showErrorMessage('Error starting FPTaylor server: ' + err, 'Copy to clipboard').then((action) => {
-			// 		if (action === 'Copy to clipboard') {
-			// 			vscode.env.clipboard.writeText(err)
-			// 		}
-			// 	})
-			// }
 		})
 	}
 
@@ -466,8 +447,7 @@ export function activate(context: vscode.ExtensionContext) {
 		const url = FPBENCH_SERVER_ADDRESS
 		// download with curl to home local share odyssey
 		const home = require('os').homedir()
-		// TODO path.join instead of string concat
-		const odysseyDir = home + '/.local/share/odyssey'
+		const odysseyDir = join(home, '.local', 'share', 'odyssey')
 		if (!fs.existsSync(odysseyDir)) {
 			fs.mkdirSync(odysseyDir, { recursive: true })
 		}
@@ -514,24 +494,6 @@ export function activate(context: vscode.ExtensionContext) {
 					}
 				})
 			}
-
-			// TODO We don't need this code, why is it here?
-			// // show information message
-			// vscode.window.showInformationMessage('FPBench installed successfully. Starting server...')
-			// try {
-			// 	// run the command in the VSCode terminal
-			// 	// show the terminal
-			// 	herbieTerminal = getTerminal()
-			// 	herbieTerminal.show()
-
-			// 	herbieTerminal.sendText(fpbenchPath + ' web --quiet')
-			// } catch (err: any) {
-			// 	vscode.window.showErrorMessage('Error starting FPBench server: ' + err, 'Copy to clipboard').then((action) => {
-			// 		if (action === 'Copy to clipboard') {
-			// 			vscode.env.clipboard.writeText(err)
-			// 		}
-			// 	})
-			// }
 		})
 	}
 
@@ -652,109 +614,6 @@ export function activate(context: vscode.ExtensionContext) {
 		}
 	}
 
-	// TODO maybe get rid of this?
-	const runFPTaylorServer = async () => {
-		try {
-			const port = 8001
-
-			const isPortFree = (port: number) =>
-				new Promise(resolve => {
-					const server = require('http')
-						.createServer()
-						.listen(port, () => {
-							server.close()
-							resolve(true)
-						})
-						.on('error', () => {
-							resolve(false)
-							return false
-						})
-				})
-			// check if port is in use
-			let somethingOnPort = !(await isPortFree(port))
-
-			if (somethingOnPort) { // yes
-				// is it FPTaylor?
-				// TODO: Figure out how to check if it's FPTaylor
-			}
-
-			// check if symlink exists
-			if (!fs.existsSync(fptaylorPath)) {
-				// wait for user to download herbie
-				vscode.window.showErrorMessage("FPTaylor doesn't seem to be installed yet. Click the button to download it.", 'Download').then((action) => {
-					if (action === 'Download') {
-						downloadFPTaylor()
-					}
-				})
-			} else if (somethingOnPort) {
-				showInfo("Using existing FPTaylor server on port " + port + ".")
-				return
-			} else {
-				herbieTerminal = getTerminal()
-				herbieTerminal.show()
-
-				// Set up FPTaylor server here
-				// TODO (rc2002)
-			}
-		} catch (err: any) {
-			vscode.window.showErrorMessage('Error starting FPTaylor server: ' + err, 'Copy to clipboard').then((action) => {
-				if (action === 'Copy to clipboard') {
-					vscode.env.clipboard.writeText(err)
-				}
-			})
-		}
-	}
-
-	// TODO maybe get rid of this?
-	const runFPBenchServer = async () => {
-		try {
-			const port = 8002
-
-			const isPortFree = (port: number) =>
-				new Promise(resolve => {
-					const server = require('http')
-						.createServer()
-						.listen(port, () => {
-							server.close()
-							resolve(true)
-						})
-						.on('error', () => {
-							resolve(false)
-							return false
-						})
-				})
-			// check if port is in use
-			let somethingOnPort = !(await isPortFree(port))
-
-			if (somethingOnPort) { // yes
-				// is it FPBench?
-				// TODO: Figure out how to check if it's FPBench
-			}
-
-			// check if symlink exists
-			if (!fs.existsSync(fpbenchPath)) {
-				// wait for user to download herbie
-				vscode.window.showErrorMessage("FPBench doesn't seem to be installed yet. Click the button to download it.", 'Download').then((action) => {
-					if (action === 'Download') {
-						downloadFPBench()
-					}
-				})
-			} else if (somethingOnPort) {
-				showInfo("Using existing FPBench server on port " + port + ".")
-				return
-			} else {
-				herbieTerminal = getTerminal()
-				herbieTerminal.show()
-			}
-		} catch (err: any) {
-			vscode.window.showErrorMessage('Error starting FPBench server: ' + err, 'Copy to clipboard').then((action) => {
-				if (action === 'Copy to clipboard') {
-					vscode.env.clipboard.writeText(err)
-				}
-			})
-		}
-	}
-
 	// read name from package.json
 	const packageJson = require('../package.json')
 	const extensionName = packageJson.name

src/herbie/ExpressionTable.tsx

@@ -245,7 +245,6 @@ function ExpressionTable() {
     // get suggested expressions with Herbie and put them in the expressions table
     const suggested = await herbiejsJobs.suggestExpressions(fpcorejs.mathjsToFPCore(expression.text, spec.expression, fpcorejs.getVarnamesMathJS(spec.expression)), sample, serverUrl)
 
-    // TODO: Fix name collision
     const derivationsObjects = suggested.derivations;
     const histories = suggested.histories;
     const alternatives = suggested.alternatives;

src/herbie/FPTaylorComponent.tsx

@@ -15,7 +15,8 @@ const FPTaylorComponent = ({ expressionId }: { expressionId: number }) => {
   }
 
   // Apply matching to automatically transform some unsupported expressions to supported ones
-  // TODO: Eventually this needs to be confirmed to FPCore parsing solution, right now it is a regex hack
+  // TODO: waiting for FPTaylor to support pow(x, 2) directly
+  // associated issue: https://github.com/herbie-fp/odyssey/issues/243
   const expression = {
     ...rawExpression,
     text: rawExpression.text.replace(/pow\((\w+),\s*2\)/g, '($1 * $1)')

src/herbie/HerbieTypes.ts

@@ -59,7 +59,6 @@ type FinalSimplifyNode = CommonFields & {
   error: number;
   "training-error": number;
   prev: DerivationNode;
-  // TODO
 };
 
 export type DerivationNode = StartNode | AddPreprocessingNode | TaylorNode | RRNode | RegimesNode | FinalSimplifyNode;