Skip to content

Commit

Permalink
repl: improve preview length calculation
Browse files Browse the repository at this point in the history
The preview had an off by one error in case colors where deactivated
and did not take fullwidth unicode characters into account when
displaying the preview.

PR-URL: #31112
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
BridgeAR authored and targos committed Apr 28, 2020
1 parent 9abe0e3 commit a892b4d
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 19 deletions.
56 changes: 39 additions & 17 deletions lib/internal/repl/utils.js
Expand Up @@ -31,26 +31,19 @@ const {
} = require('readline');

const {
commonPrefix
commonPrefix,
getStringWidth,
} = require('internal/readline/utils');

const { inspect } = require('util');

const debug = require('internal/util/debuglog').debuglog('repl');

const inspectOptions = {
depth: 1,
const previewOptions = {
colors: false,
compact: true,
breakLength: Infinity
};
// Specify options that might change the output in a way that it's not a valid
// stringified object anymore.
const inspectedOptions = inspect(inspectOptions, {
depth: 1,
colors: false,
showHidden: false
});
};

// If the error is that we've unexpectedly ended the input,
// then let the user try to recover by adding more input.
Expand Down Expand Up @@ -254,7 +247,7 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
}
const { result } = preview;
if (result.value !== undefined) {
callback(null, inspect(result.value, inspectOptions));
callback(null, inspect(result.value, previewOptions));
// Ignore EvalErrors, SyntaxErrors and ReferenceErrors. It is not clear
// where they came from and if they are recoverable or not. Other errors
// may be inspected.
Expand All @@ -264,8 +257,19 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
result.className === 'ReferenceError')) {
callback(null, null);
} else if (result.objectId) {
// The writer options might change and have influence on the inspect
// output. The user might change e.g., `showProxy`, `getters` or
// `showHidden`. Use `inspect` instead of `JSON.stringify` to keep
// `Infinity` and similar intact.
const inspectOptions = inspect({
...repl.writer.options,
colors: false,
depth: 1,
compact: true,
breakLength: Infinity
}, previewOptions);
session.post('Runtime.callFunctionOn', {
functionDeclaration: `(v) => util.inspect(v, ${inspectedOptions})`,
functionDeclaration: `(v) => util.inspect(v, ${inspectOptions})`,
objectId: result.objectId,
arguments: [result]
}, (error, preview) => {
Expand Down Expand Up @@ -337,15 +341,33 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
// Limit the output to maximum 250 characters. Otherwise it becomes a)
// difficult to read and b) non terminal REPLs would visualize the whole
// output.
const maxColumns = MathMin(repl.columns, 250);

if (inspected.length > maxColumns) {
inspected = `${inspected.slice(0, maxColumns - 6)}...`;
let maxColumns = MathMin(repl.columns, 250);

// Support unicode characters of width other than one by checking the
// actual width.
// TODO(BridgeAR): Add a test case to verify full-width characters work as
// expected. Also test that the line break in case of deactivated colors
// work as expected.
if (inspected.length * 2 >= maxColumns &&
getStringWidth(inspected) > maxColumns) {
maxColumns -= 4 + (repl.useColors ? 0 : 3);
let res = '';
for (const char of inspected) {
maxColumns -= getStringWidth(char);
if (maxColumns < 0)
break;
res += char;
}
inspected = `${res}...`;
}

// Line breaks are very rare and probably only occur in case of error
// messages with line breaks.
const lineBreakPos = inspected.indexOf('\n');
if (lineBreakPos !== -1) {
inspected = `${inspected.slice(0, lineBreakPos)}`;
}

const result = repl.useColors ?
`\u001b[90m${inspected}\u001b[39m` :
`// ${inspected}`;
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-repl-history-navigation.js
Expand Up @@ -85,7 +85,7 @@ const tests = [
'144, 169, 196, 225, 256, 289, 324, 361, 400, 441, 484, 529,' +
' 576, 625, 676, 729, 784, 841, 900, 961, 1024, 1089, 1156, ' +
'1225, 1296, 1369, 1444, 1521, 1600, 1681, 1764, 1849, 1936,' +
' 2025, 2116, 2209, ...',
' 2025, 2116, 2209,...',
`${prompt}{key : {key2 :[] }}`,
prev && '\n// { key: { key2: [] } }',
`${prompt}555 + 909`,
Expand All @@ -100,7 +100,7 @@ const tests = [
'144, 169, 196, 225, 256, 289, 324, 361, 400, 441, 484, 529,' +
' 576, 625, 676, 729, 784, 841, 900, 961, 1024, 1089, 1156, ' +
'1225, 1296, 1369, 1444, 1521, 1600, 1681, 1764, 1849, 1936,' +
' 2025, 2116, 2209, ...',
' 2025, 2116, 2209,...',
prompt].filter((e) => typeof e === 'string'),
clean: true
},
Expand Down

0 comments on commit a892b4d

Please sign in to comment.