fixed not working cmd command for uploading and executing local scripts #5
Conversation
|
@bkircher please have a look |
wehrstedt
force-pushed
the
cmd-command-fix
branch
from
263b8ed to
582a788
Compare
|
there were two semicolons... |
src/cmd.ts
Outdated
| // console.log('test ' + dir_i); | ||
| // }); | ||
| .action(function (json: string, dir: string[]) { | ||
| console.log("Note:\nIf you you use wildcards in the path to the dir to upload, set the path in quoatmarks.\n"); |
src/cmd.ts
Outdated
| @@ -13,27 +14,43 @@ var program = require('commander'); | |||
| // set up sdsAccess | |||
|
|
|||
|
|
|||
| async function uploadAndRunAll(loginData: config.LoginData, folder: string, prefix: string): Promise<sdsAccess.scriptT[]> { | |||
| async function uploadAndRunAll(loginData: config.LoginData, files: sdsAccess.scriptT[]): Promise<sdsAccess.scriptT[]> { | |||
There was a problem hiding this comment.
You change the API here. What implications has this for consumers of this library?
There was a problem hiding this comment.
I'm pretty sure that the whole script actually was implemented for me to execute and upload scripts from the command line. Also, this function is not exported, so the only place where this function can be used is in the same script. But you're right, it would be better to split this into multiple functions.
There was a problem hiding this comment.
Okay. Since you're the only one using it and it is not exported the change is fine. 👍
src/cmd.ts
Outdated
| function readDirSync(dir: string, rec: boolean = true): string[] { | ||
| var results: string[] = []; | ||
| var list = fs.readdirSync(dir); | ||
|
|
There was a problem hiding this comment.
Use let instead of var. Even better const.
src/cmd.ts
Outdated
| // uploadAndRunAll(loginData, dirToUpload, execPrefix); | ||
| // } else { | ||
| // console.log('Dir/file to run/upload missing'); | ||
| // } | ||
| }); |
There was a problem hiding this comment.
You should not commit commented-out lines. Just remove them. Use git for version control.
There was a problem hiding this comment.
whoops, forgot to remove these lines 🙈
|
You have to execute the following command: node src/out/cmd.js test <path-to-launch.json> I will add this to the documentation tomorrow. |
Great! Can you also push one or more commits with the requested changes to this PR ? Then I will test and merge. |
wehrstedt
force-pushed
the
cmd-command-fix
branch
from
582a788 to
6481b09
Compare
|
sorry, i made a lot of changes and splitted the changes into multiple functions. Now there are two more cmd-commands for upload and execute scripts from the command line. I didn't changed the old cmd command, just fixed the issue there so that this command is now ready to use. |
|
can you tag the changes if this PR is fine? |
Fixed the not working cmd command tool.
Added recursive file searching.
Improved the wildcard resolving of file paths.