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

repl: Support for eager evaluation #22875

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
95 changes: 94 additions & 1 deletion lib/internal/repl/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ const numericSeparator =
const staticClassFeatures =
require('internal/deps/acorn-plugins/acorn-static-class-features/index');
const { tokTypes: tt, Parser: AcornParser } = acorn;
const util = require('util');
const debug = require('internal/util/debuglog').debuglog('repl');

// 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 @@ -87,7 +89,98 @@ function isRecoverableError(e, code) {
}
}

// Appends the preview of the result
// to the tty.
Comment on lines +92 to +93
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think this will fit on one line.

function appendPreview(repl, result, cursorTo, clearScreenDown) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function appendPreview(repl, result, cursorTo, clearScreenDown) {
function appendPreview(repl, preview, cursorTo, clearScreenDown) {

maybe? To make it a little clearer.

repl.previewResult = `\u001b[90m // ${result}\u001b[39m`;
const line = `${repl._prompt}${repl.line} //${result}`;
const columns = repl.output.columns;
const hasColors = repl.output.hasColors ? repl.output.hasColors() : false;
const s = hasColors ?
`${repl._prompt}${repl.line}${repl.previewResult}` : line;

// Cursor to left edge.
cursorTo(repl.output, 0);
clearScreenDown(repl.output);

if (columns !== undefined) {
repl.output.write(line.length < columns ?
s : `${s.slice(0, columns - 3)
.replace(/\r?\n|\r/g, '')}...\u001b[39m`);
Comment on lines +107 to +109
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

    repl.output.write(line.length < columns ?
      s : s.slice(0, columns - 3).replace(/\r?\n|\r/g, '') + '...\u001b[39m');

} else {
repl.output.write(s);
}

// Move back the cursor to the original position
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
// Move back the cursor to the original position
// Move back the cursor to the original position.

cursorTo(repl.output, repl.cursor + repl._prompt.length);
}

function clearPreview(repl) {
if (repl.previewResult !== '') {
repl._refreshLine();
repl.previewResult = '';
}
}

// Called whenever a line changes
// in repl and the eager eval will be
// executed against the line using v8 session
let readline;
function makePreview(repl, eagerSession, eagerEvalContextId, line) {
const lazyReadline = () => {
if (!readline) readline = require('readline');
return readline;
};

const { cursorTo, clearScreenDown } = lazyReadline();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 can we just const { cursorTo, clearScreenDown } = require('readline') at the top now?


clearPreview(repl);

eagerSession.post('Runtime.evaluate', {
expression: line.toString(),
generatePreview: true,
throwOnSideEffect: true,
timeout: 500,
executionContextId: eagerEvalContextId
}, (error, previewResult) => {

if (error) {
debug(`Error while generating preview ${error}`);
return;
}

if (undefined !== previewResult.result.value) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
if (undefined !== previewResult.result.value) {
if (previewResult.result.value !== undefined) {

const value = util.inspect(previewResult.result.value);
appendPreview(repl, value, cursorTo, clearScreenDown);
return;
}


// If there is no exception and we got
// objectId in the result, stringify it
// using inspect via Runtime.callFunctionOn
if (!previewResult.exceptionDetails && previewResult.result.objectId) {
eagerSession.post('Runtime.callFunctionOn', {
functionDeclaration:
'function(arg) { return util.inspect(arg) }',
arguments: [previewResult.result],
executionContextId: eagerEvalContextId,
returnByValue: true,
}, (err, result) => {
if (!err) {
appendPreview(repl, result.result.value,
cursorTo, clearScreenDown);
} else {
debug('eager eval error', err);
}
});
}
});
}


module.exports = {
isRecoverableError,
kStandaloneREPL: Symbol('kStandaloneREPL')
kStandaloneREPL: Symbol('kStandaloneREPL'),
makePreview
};
9 changes: 9 additions & 0 deletions lib/readline.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ function Interface(input, output, completer, terminal) {
this.output = output;
this.input = input;
this.historySize = historySize;
this.previewResult = '';
this.removeHistoryDuplicates = !!removeHistoryDuplicates;
this.crlfDelay = crlfDelay ?
MathMax(kMincrlfDelay, crlfDelay) : kMincrlfDelay;
Expand Down Expand Up @@ -490,6 +491,8 @@ Interface.prototype._insertString = function(c) {
// A hack to get the line refreshed if it's needed
this._moveCursor(0);
}
// Emit current line for generating preview
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
// Emit current line for generating preview
// Emit current line for generating preview.

this.emit('buffer', this.line);
};

Interface.prototype._tabComplete = function(lastKeypressWasTab) {
Expand Down Expand Up @@ -629,6 +632,7 @@ Interface.prototype._deleteLeft = function() {

this.cursor -= charSize;
this._refreshLine();
this.emit('buffer', this.line);
}
};

Expand All @@ -640,6 +644,7 @@ Interface.prototype._deleteRight = function() {
this.line = this.line.slice(0, this.cursor) +
this.line.slice(this.cursor + charSize, this.line.length);
this._refreshLine();
this.emit('buffer', this.line);
}
};

Expand All @@ -655,6 +660,7 @@ Interface.prototype._deleteWordLeft = function() {
this.line = leading + this.line.slice(this.cursor, this.line.length);
this.cursor = leading.length;
this._refreshLine();
this.emit('buffer', this.line);
}
};

Expand All @@ -666,6 +672,7 @@ Interface.prototype._deleteWordRight = function() {
this.line = this.line.slice(0, this.cursor) +
trailing.slice(match[0].length);
this._refreshLine();
this.emit('buffer', this.line);
}
};

Expand All @@ -674,12 +681,14 @@ Interface.prototype._deleteLineLeft = function() {
this.line = this.line.slice(this.cursor);
this.cursor = 0;
this._refreshLine();
this.emit('buffer', this.line);
};


Interface.prototype._deleteLineRight = function() {
this.line = this.line.slice(0, this.cursor);
this._refreshLine();
this.emit('buffer', this.line);
};


Expand Down
18 changes: 17 additions & 1 deletion lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ const experimentalREPLAwait = require('internal/options').getOptionValue(
);
const {
isRecoverableError,
kStandaloneREPL
kStandaloneREPL,
makePreview
} = require('internal/repl/utils');
const {
getOwnNonIndexProperties,
Expand All @@ -111,6 +112,7 @@ const {

const history = require('internal/repl/history');
const { setImmediate } = require('timers');
const inspector = require('inspector');

// Lazy-loaded.
let processTopLevelAwait;
Expand Down Expand Up @@ -223,6 +225,20 @@ function REPLServer(prompt,
throw new ERR_INVALID_REPL_EVAL_CONFIG();
}

const eagerSession = new inspector.Session();
eagerSession.connect();
eagerSession.once('Runtime.executionContextCreated',
({ params: { context } }) => {
Comment on lines +230 to +231
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps to avoid big indentation:

eagerSession.once(
  'Runtime.executionContextCreated',
  ({ params: { context } }) => {
  
  })

this.on('buffer', (line) => {
// No need of preview for a multiline statement.
if (this[kBufferedCommandSymbol] !== '')
return;
makePreview(self, eagerSession, context.id, line);
});
eagerSession.post('Runtime.disable');
});
eagerSession.post('Runtime.enable');

// Add this listener only once and use a WeakSet that contains the REPLs
// domains. Otherwise we'd have to add a single listener to each REPL instance
// and that could trigger the `MaxListenersExceededWarning`.
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-repl-persistent-history.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ const tests = [
{
env: {},
test: [UP, '\'42\'', ENTER],
expected: [prompt, '\'', '4', '2', '\'', '\'42\'\n', prompt, prompt],
expected: [prompt, '\'', '4', '2', '\'',
'> \'42\' //\'42\'', '\'42\'\n',
prompt, prompt],
clean: false
},
{ // Requires the above test case
Expand Down
152 changes: 152 additions & 0 deletions test/parallel/test-repl-preview-result.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
'use strict';

// Flags: --expose-internals

const common = require('../common');
const stream = require('stream');
const REPL = require('internal/repl');
const assert = require('assert');

// Create an input stream specialized for testing an array of actions
class ActionStream extends stream.Stream {
run(data) {
const _iter = data[Symbol.iterator]();
const doAction = () => {
const next = _iter.next();
if (next.done) {
// Close the repl. Note that it must have a clean prompt to do so.
setImmediate(() => {
this.emit('keypress', '', { ctrl: true, name: 'd' });
});
return;
}
const action = next.value;

if (typeof action === 'object') {
this.emit('keypress', '', action);
} else {
this.emit('data', `${action}\n`);
}
setImmediate(doAction);
};
setImmediate(doAction);
}
resume() {}
pause() {}
}
ActionStream.prototype.readable = true;


// Mock keys
const ENTER = { name: 'enter' };
const CLEAR = { ctrl: true, name: 'u' };

const prompt = '> ';


const wrapWithColorCode = (code, result) => {
return `${prompt}${code}\u001b[90m // ${result}\u001b[39m`;
};
const tests = [
{
env: {},
test: ['\' t\'.trim()', CLEAR],
expected: [wrapWithColorCode('\' t\'', '\' t\''),
wrapWithColorCode('\' t\'.trim', '[Function: trim]'),
wrapWithColorCode('\' t\'.trim()', '\'t\'')]
},
{
env: {},
test: ['3+5', CLEAR],
expected: [wrapWithColorCode('3', '3'),
wrapWithColorCode('3+5', '8')]
},
{
env: {},
test: ['[9,0].sort()', CLEAR],
expected: [wrapWithColorCode('[9,0]', '[ 9, 0 ]'),
wrapWithColorCode('[9,0].sort', '[Function: sort]'),
wrapWithColorCode('[9,0].sort()', '[ 0, 9 ]')]
},
{
env: {},
test: ['const obj = { m : () => {}}', ENTER,
'obj.m', CLEAR],
expected: [
wrapWithColorCode('obj', '{ m: [Function: m] }'),
wrapWithColorCode('obj.m', '[Function: m]')]
},
{
env: {},
test: ['const aObj = { a : { b : { c : [ {} , \'test\' ]}}}', ENTER,
'aObj.a', CLEAR],
expected: [
wrapWithColorCode('aObj',
'{ a: { b: { c: [Array] } } }'),
wrapWithColorCode('aObj.a',
'{ b: { c: [ {}, \'test\' ] } }')]
}
];
const numtests = tests.length;

const runTestWrap = common.mustCall(runTest, numtests);

function runTest() {
const opts = tests.shift();
if (!opts) return; // All done

const env = opts.env;
const test = opts.test;
const expected = opts.expected;
const ouput = new stream.Writable({
write(chunk, _, next) {
const output = chunk.toString();

// Ignore everything except eval result
if (!output.includes('//')) {
return next();
}

const toBeAsserted = expected[0];
try {
assert.strictEqual(output, toBeAsserted);
expected.shift();
} catch (err) {
console.error(`Failed test # ${numtests - tests.length}`);
throw err;
}

next();
},
});
ouput.hasColors = () => true;

REPL.createInternalRepl(env, {
input: new ActionStream(),
output: ouput,
prompt: prompt,
useColors: false,
terminal: true
}, function(err, repl) {
if (err) {
console.error(`Failed test # ${numtests - tests.length}`);
throw err;
}

repl.once('close', () => {
try {
// Ensure everything that we expected was output
assert.strictEqual(expected.length, 0);
setImmediate(runTestWrap, true);
} catch (err) {
console.error(`Failed test # ${numtests - tests.length}`);
throw err;
}
});

repl.inputStream.run(test);
});
}

// run the tests
runTest();
4 changes: 3 additions & 1 deletion test/parallel/test-repl-programmatic-history.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ const tests = [
{
env: {},
test: [UP, '\'42\'', ENTER],
expected: [prompt, '\'', '4', '2', '\'', '\'42\'\n', prompt, prompt],
expected: [prompt, '\'', '4', '2', '\'',
`${prompt}'42' //'42'`,
'\'42\'\n', prompt, prompt],
clean: false
},
{ // Requires the above test case
Expand Down