Require to use "-" when reading from stdin#40424
Conversation
bpasero
added
the
candidate
| }, 1000); | ||
|
|
||
| // ...but finish early if we detect data | ||
| process.stdin.on('data', () => { |
There was a problem hiding this comment.
Should this be once instead of on?
There was a problem hiding this comment.
Also, should the listener be detached when this promise resolves?
| } | ||
|
|
||
| export const optionsHelp: { [name: string]: string; } = { | ||
| '-': isWindows ? localize('stdinWindows', "Read output from another program (e.g. 'echo Hello World | {0} -').", product.applicationName) : localize('stdinUnix', "Read from stdin (e.g. 'ps aux | grep code | {0} -').", product.applicationName), |
There was a problem hiding this comment.
- isn't an option, I wouldn't put it here since options can be put in any position of the CLI. - seems to be only placeable where files are. I would either omit this advanced option in the help message, or simply document it further down: instead of a file path you can also use - to read input from stdin. Something like that.
| argv.push('--skip-add-to-recently-opened'); | ||
| args.wait = true; | ||
| } catch (error) { | ||
| stdinFileError = error; |
There was a problem hiding this comment.
The only code which will throw is on line 105: const stdinFileStream.... Should that line be the only one wrapped in a try catch block?
| let stdinFileError: Error; | ||
| try { | ||
| const stdinFileStream = fs.createWriteStream(stdinFilePath); | ||
| resolveTerminalEncoding(verbose).done(encoding => { |
There was a problem hiding this comment.
What happens in this promise's error case?
There was a problem hiding this comment.
@joaomoreno actually resolveTerminalEncoding is never returning with an error
* Require to use "-" when reading from stdin * cli - better help message * more narrow error handling * better data listener
2 participants
fixes #40351