Require to use "-" when reading from stdin (#40424)

* Require to use "-" when reading from stdin

* cli - better help message

* more narrow error handling

* better data listener

src/vs/code/node/cli.ts

@@ -115,17 +115,15 @@ export async function main(argv: string[]): TPromise<any> {
 	// Just Code
 	else {
 		const env = assign({}, process.env, {
-			// this will signal Code that it was spawned from this module
-			'VSCODE_CLI': '1',
+			'VSCODE_CLI': '1', // this will signal Code that it was spawned from this module
 			'ELECTRON_NO_ATTACH_CONSOLE': '1'
 		});
 
 		delete env['ELECTRON_RUN_AS_NODE'];
 
-		let processCallbacks: ((child: ChildProcess) => Thenable<any>)[] = [];
+		const processCallbacks: ((child: ChildProcess) => Thenable<any>)[] = [];
 
 		const verbose = args.verbose || args.status;
-
 		if (verbose) {
 			env['ELECTRON_ENABLE_LOGGING'] = '1';
 
@@ -137,46 +135,87 @@ export async function main(argv: string[]): TPromise<any> {
 			});
 		}
 
-		// If we are running with input from stdin, pipe that into a file and
-		// open this file via arguments. Ignore this when we are passed with
-		// paths to open.
-		let isReadingFromStdin: boolean;
+		let stdinWithoutTty: boolean;
 		try {
-			isReadingFromStdin = args._.length === 0 && !process.stdin.isTTY; // Via https://twitter.com/MylesBorins/status/782009479382626304
+			stdinWithoutTty = !process.stdin.isTTY; // Via https://twitter.com/MylesBorins/status/782009479382626304
 		} catch (error) {
 			// Windows workaround for https://github.com/nodejs/node/issues/11656
 		}
 
 		let stdinFilePath: string;
-		if (isReadingFromStdin) {
-			let stdinFileError: Error;
-			stdinFilePath = paths.join(os.tmpdir(), `code-stdin-${Math.random().toString(36).replace(/[^a-z]+/g, '').substr(0, 3)}.txt`);
-			try {
-				const stdinFileStream = fs.createWriteStream(stdinFilePath);
-				resolveTerminalEncoding(verbose).done(encoding => {
+		if (stdinWithoutTty) {
+
+			// Read from stdin: we require a single "-" argument to be passed in order to start reading from
+			// stdin. We do this because there is no reliable way to find out if data is piped to stdin. Just
+			// checking for stdin being connected to a TTY is not enough (https://github.com/Microsoft/vscode/issues/40351)
+			if (args._.length === 1 && args._[0] === '-') {
+
+				// remove the "-" argument when we read from stdin
+				args._ = [];
+				argv = argv.filter(a => a !== '-');
+
+				// prepare temp file to read stdin to
+				stdinFilePath = paths.join(os.tmpdir(), `code-stdin-${Math.random().toString(36).replace(/[^a-z]+/g, '').substr(0, 3)}.txt`);
+
+				// open tmp file for writing
+				let stdinFileError: Error;
+				let stdinFileStream: fs.WriteStream;
+				try {
+					stdinFileStream = fs.createWriteStream(stdinFilePath);
+				} catch (error) {
+					stdinFileError = error;
+				}
 
-					// Pipe into tmp file using terminals encoding
-					const converterStream = iconv.decodeStream(encoding);
-					process.stdin.pipe(converterStream).pipe(stdinFileStream);
-				});
+				if (!stdinFileError) {
 
-				// Make sure to open tmp file
-				argv.push(stdinFilePath);
+					// Pipe into tmp file using terminals encoding
+					resolveTerminalEncoding(verbose).done(encoding => {
+						const converterStream = iconv.decodeStream(encoding);
+						process.stdin.pipe(converterStream).pipe(stdinFileStream);
+					});
+
+					// Make sure to open tmp file
+					argv.push(stdinFilePath);
+
+					// Enable --wait to get all data and ignore adding this to history
+					argv.push('--wait');
+					argv.push('--skip-add-to-recently-opened');
+					args.wait = true;
+				}
 
-				// Enable --wait to get all data and ignore adding this to history
-				argv.push('--wait');
-				argv.push('--skip-add-to-recently-opened');
-				args.wait = true;
-			} catch (error) {
-				stdinFileError = error;
+				if (verbose) {
+					if (stdinFileError) {
+						console.error(`Failed to create file to read via stdin: ${stdinFileError.toString()}`);
+					} else {
+						console.log(`Reading from stdin via: ${stdinFilePath}`);
+					}
+				}
 			}
 
-			if (verbose) {
-				if (stdinFileError) {
-					console.error(`Failed to create file to read via stdin: ${stdinFileError.toString()}`);
-				} else {
-					console.log(`Reading from stdin via: ${stdinFilePath}`);
-				}
+			// If the user pipes data via stdin but forgot to add the "-" argument, help by printing a message
+			// if we detect that data flows into via stdin after a certain timeout.
+			else if (args._.length === 0) {
+				processCallbacks.push(child => new TPromise(c => {
+					const dataListener = () => {
+						if (isWindows) {
+							console.log(`Run with '${product.applicationName} -' to read output from another program (e.g. 'echo Hello World | ${product.applicationName} -').`);
+						} else {
+							console.log(`Run with '${product.applicationName} -' to read from stdin (e.g. 'ps aux | grep code | ${product.applicationName} -').`);
+						}
+
+						c(void 0);
+					};
+
+					// wait for 1s maximum...
+					setTimeout(() => {
+						process.stdin.removeListener('data', dataListener);
+
+						c(void 0);
+					}, 1000);
+
+					// ...but finish early if we detect data
+					process.stdin.once('data', dataListener);
+				}));
 			}
 		}
 

src/vs/platform/environment/node/argv.ts

@@ -9,6 +9,8 @@ import * as assert from 'assert';
 import { firstIndex } from 'vs/base/common/arrays';
 import { localize } from 'vs/nls';
 import { ParsedArgs } from '../common/environment';
+import { isWindows } from 'vs/base/common/platform';
+import product from 'vs/platform/node/product';
 
 const options: minimist.Opts = {
 	string: [
@@ -199,6 +201,8 @@ export function buildHelpMessage(fullName: string, name: string, version: string
 
 ${ localize('usage', "Usage")}: ${executable} [${localize('options', "options")}] [${localize('paths', 'paths')}...]
 
+${ isWindows ? localize('stdinWindows', "To read output from another program, append '-' (e.g. 'echo Hello World | {0} -')", product.applicationName) : localize('stdinUnix', "To read from stdin, append '-' (e.g. 'ps aux | grep code | {0} -')", product.applicationName)}
+
 ${ localize('optionsUpperCase', "Options")}:
 ${formatOptions(optionsHelp, columns)}`;
 }