Sdfix #45
Sdfix #45
Conversation
src/matlab.ts
Outdated
| @@ -54,7 +54,7 @@ export async function runCommand(hs: HelperScript, platform: string, architectur | |||
| const rmcPath = getRunMATLABCommandScriptPath(platform, architecture); | |||
| await fs.chmod(rmcPath, 0o777); | |||
|
|
|||
| const rmcArg = script.cdAndCall(hs.dir, hs.command); | |||
| const rmcArg = `setenv("MW_ORIG_WORKING_FOLDER", cd('${hs.dir}'));${hs.command}`; | |||
There was a problem hiding this comment.
Safest to use ' instead of " to support MATLAB releases as far back as possible.
Also probably a good idea to escape any ' chars in hs.dir.
src/script.ts
Outdated
| export function cdAndCall(dir: string, command: string): string { | ||
| return `cd('${pathToCharVec(dir)}');${command}`; | ||
| export function cdAndCall(command: string): string { | ||
| return `cd(getenv("MW_ORIG_WORKING_FOLDER"));${command}`; |
src/script.unit.test.ts
Outdated
| const testCommand = "disp('hello world')"; | ||
| const expectedString = String.raw`cd('C:\Users\you\You''re Documents');${testCommand}`; | ||
| const expectedString = `cd(getenv("MW_ORIG_WORKING_FOLDER"));${testCommand}`; |
src/matlab.ts
Outdated
| @@ -54,7 +57,7 @@ export async function runCommand(hs: HelperScript, platform: string, architectur | |||
| const rmcPath = getRunMATLABCommandScriptPath(platform, architecture); | |||
| await fs.chmod(rmcPath, 0o777); | |||
|
|
|||
| const rmcArg = script.cdAndCall(hs.dir, hs.command); | |||
| const rmcArg = `setenv('MW_ORIG_WORKING_FOLDER', cd('${hs.dir}'));${hs.command}`; | |||
There was a problem hiding this comment.
Probably better to escape 's here right before injecting it into the char array. Then all users of this function don't need to know they need to escape the dir before calling this function and the need to escape is implementation detail of this function.
There was a problem hiding this comment.
Wouldn't that mean that the other function consumers would likely have to escape the dir themselves later on? I can't think of many cases where someone would be calling generateScript and not wanting the directory to be escaped but i'm probably missing something
There was a problem hiding this comment.
generateScript just generates a script and returns the directory that script lives in. It shouldn't assume you're then going to inject that directory path into a MATLAB char array later on. If, for instance, you wanted to delete that directory from JavaScript code, you've got an issue because the directory path is wrong.
Fix for the
-sdoption using an environment variable. Because it changes the function signature ofcdAndCall, run-build and run-tests will need to be updated in parallel.EDIT: Even better it looks like I remembered incorrectly and the other two actions don't directly call
cdAndCall