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>
BridgeAR authored and MylesBorins committed Jan 16, 2020
1 parent de6f2be commit d84c394
Showing 2 changed files with 41 additions and 19 deletions.
56 changes: 39 additions & 17 deletions lib/internal/repl/utils.js
Original file line number Diff line number Diff line change
@@ -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.
@@ -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.
@@ -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) => {
@@ -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}`;
4 changes: 2 additions & 2 deletions test/parallel/test-repl-history-navigation.js
Original file line number Diff line number Diff line change
@@ -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`,
@@ -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
},

0 comments on commit d84c394

Please sign in to comment.