Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

readline,repl: add substring history search #31112

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
repl: improve preview length calculation
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.
  • Loading branch information
BridgeAR committed Jan 8, 2020
commit 05c19bdbf0d9c4d78725749320655c363d362b5a
56 changes: 39 additions & 17 deletions lib/internal/repl/utils.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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