Skip to content

Commit f1f5afc

Browse files
authored
chore: clean up color/tty handling core in cli-repl (#387)
Don’t assume that `process.stdout.isTTY` is a good indicator for coloring strings: - Not all terminals have color support - We should honour environment variables like `NO_COLOR` - Not all formatted strings end up printed to stdout Also, don’t tell the Node.js repl that we are working with a terminal if we don’t know so for sure.
1 parent 7f77c58 commit f1f5afc

File tree

7 files changed

+314
-271
lines changed

7 files changed

+314
-271
lines changed

packages/cli-repl/src/arg-parser.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { CliOptions } from '@mongosh/service-provider-server';
22
import { USAGE } from './constants';
33
import i18n from '@mongosh/i18n';
44
import minimist from 'minimist';
5-
import clr from './clr';
5+
import { colorizeForStderr as clr } from './clr';
66

77
/**
88
* Unknown translation key.

packages/cli-repl/src/cli-repl.ts

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ class CliRepl {
9797
*/
9898
async connect(driverUri: string, driverOptions: NodeOptions): Promise<any> {
9999
if (!this.options.nodb) {
100-
console.log(i18n.__(CONNECTING), ' ', clr(retractPassword(driverUri), ['bold', 'green']));
100+
console.log(i18n.__(CONNECTING), ' ', this.clr(retractPassword(driverUri), ['bold', 'green']));
101101
}
102102
return await CliServiceProvider.connect(driverUri, driverOptions, this.options);
103103
}
@@ -116,9 +116,9 @@ class CliRepl {
116116
prompt: '> ',
117117
writer: this.writer,
118118
completer: completer.bind(null, version),
119-
terminal: true,
120119
breakEvalOnSigint: true,
121120
preview: false,
121+
terminal: process.env.MONGOSH_FORCE_TERMINAL ? true : undefined
122122
});
123123

124124
const originalDisplayPrompt = this.repl.displayPrompt.bind(this.repl);
@@ -128,12 +128,14 @@ class CliRepl {
128128
this.lineByLineInput.nextLine();
129129
};
130130

131-
const originalEditorAction = this.repl.commands.editor.action.bind(this.repl);
131+
if (this.repl.commands.editor) {
132+
const originalEditorAction = this.repl.commands.editor.action.bind(this.repl);
132133

133-
this.repl.commands.editor.action = (): any => {
134-
this.lineByLineInput.disableBlockOnNewline();
135-
return originalEditorAction();
136-
};
134+
this.repl.commands.editor.action = (): any => {
135+
this.lineByLineInput.disableBlockOnNewline();
136+
return originalEditorAction();
137+
};
138+
}
137139

138140
this.repl.defineCommand('clear', {
139141
help: '',
@@ -355,10 +357,10 @@ class CliRepl {
355357
stack: result.stack
356358
};
357359
this.bus.emit('mongosh:error', output);
358-
return formatOutput({ type: 'Error', value: output });
360+
return this.formatOutput({ type: 'Error', value: output });
359361
}
360362

361-
return formatOutput({ type: result.type, value: result.printable });
363+
return this.formatOutput({ type: result.type, value: result.printable });
362364
};
363365

364366
verifyNodeVersion(): void {
@@ -377,7 +379,7 @@ class CliRepl {
377379
greet(): void {
378380
const { version } = require('../package.json');
379381
console.log(`Using MongoDB: ${this.internalState.connectionInfo.buildInfo.version}`);
380-
console.log(`${clr('Using Mongosh Beta', ['bold', 'yellow'])}: ${version}`);
382+
console.log(`${this.clr('Using Mongosh Beta', ['bold', 'yellow'])}: ${version}`);
381383
console.log(`${MONGOSH_WIKI}`);
382384
if (!this.disableGreetingMessage) console.log(TELEMETRY);
383385
}
@@ -410,7 +412,7 @@ class CliRepl {
410412
read(readOptions, (error, password) => {
411413
if (error) {
412414
this.bus.emit('mongosh:error', error);
413-
return console.log(formatError(error));
415+
return console.log(this.formatError(error));
414416
}
415417

416418
driverOptions.auth.password = password;
@@ -465,7 +467,7 @@ class CliRepl {
465467
this.bus.emit('mongosh:error', error);
466468
}
467469

468-
console.error(formatError(error));
470+
console.error(this.formatError(error));
469471
return process.exit(1);
470472
}
471473

@@ -475,6 +477,25 @@ class CliRepl {
475477
// https://github.com/DefinitelyTyped/DefinitelyTyped/pull/48646
476478
(this.repl as any).output.write(joined + '\n');
477479
}
480+
481+
formatOutput(value: any): string {
482+
return formatOutput(value, this.getFormatOptions());
483+
}
484+
485+
formatError(value: any): string {
486+
return formatError(value, this.getFormatOptions());
487+
}
488+
489+
clr(text: string, style: string|string[]): string {
490+
return clr(text, style, this.getFormatOptions());
491+
}
492+
493+
getFormatOptions(): { colors: boolean } {
494+
return {
495+
colors: this.repl ? this.repl.useColors :
496+
process.stdout.isTTY && process.stdout.getColorDepth() > 1
497+
};
498+
}
478499
}
479500

480501
export default CliRepl;

packages/cli-repl/src/clr.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
import ansi from 'ansi-escape-sequences';
22

3-
export default function clr(text: string, style: any): string {
4-
return process.stdout.isTTY ? ansi.format(text, style) : text;
3+
export default function colorize(text: string, style: string|string[], options: { colors: boolean }): string {
4+
if (options.colors) {
5+
return ansi.format(text, style);
6+
}
7+
return text;
8+
}
9+
10+
export function colorizeForStdout(text: string, style: string|string[]): string {
11+
return colorize(text, style, {
12+
colors: process.stdout.isTTY && process.stdout.getColorDepth() > 1 });
13+
}
14+
15+
export function colorizeForStderr(text: string, style: string|string[]): string {
16+
return colorize(text, style, {
17+
colors: process.stderr.isTTY && process.stderr.getColorDepth() > 1 });
518
}

packages/cli-repl/src/constants.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import i18n from '@mongosh/i18n';
2-
import clr from './clr';
2+
import { colorizeForStderr as clr } from './clr';
33

44
export const TELEMETRY = `
55
${i18n.__('cli-repl.cli-repl.telemetry')}

0 commit comments

Comments
 (0)