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: Add editor mode support #7275

Closed
wants to merge 6 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
15 changes: 15 additions & 0 deletions doc/api/repl.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@ The following special commands are supported by all REPL instances:
`> .save ./file/to/save.js`
* `.load` - Load a file into the current REPL session.
`> .load ./file/to/load.js`
* `.editor` - Enter editor mode (`<ctrl>-D` to finish, `<ctrl>-C` to cancel)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add a snippet of an example?


```js
> .editor
// Entering editor mode (^D to finish, ^C to cancel)
function welcome(name) {
return `Hello ${name}!`;
}

welcome('Node.js User');

// ^D
'Hello Node.js User!'
>
```

The following key combinations in the REPL have these special effects:

Expand Down
117 changes: 114 additions & 3 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ function REPLServer(prompt,
self.underscoreAssigned = false;
self.last = undefined;
self.breakEvalOnSigint = !!breakEvalOnSigint;
self.editorMode = false;

self._inTemplateLiteral = false;

Expand Down Expand Up @@ -394,7 +395,12 @@ function REPLServer(prompt,
// Figure out which "complete" function to use.
self.completer = (typeof options.completer === 'function')
? options.completer
: complete;
: completer;

function completer(text, cb) {
complete.call(self, text, self.editorMode
? self.completeOnEditorMode(cb) : cb);
}

Interface.call(this, {
input: self.inputStream,
Expand Down Expand Up @@ -428,9 +434,11 @@ function REPLServer(prompt,
});

var sawSIGINT = false;
var sawCtrlD = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we use self.editorMode itself? Is it really necessary to have sawCtrlD?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thefourtheye sawCtrlD is set to true after turnOffEditorMode. we can execute code only when the editor mode is off

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, when turnOffEditorMode is called, it sets self.editorMode to false. Why can't that itself be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

self.editorMode flag tracks the editor mode where as sawCtrlD indicates that editor mode is turned off and is not canceled.
sawCtrlD flag ensures that vm returned error is non recoverable.

self.on('SIGINT', function() {
var empty = self.line.length === 0;
self.clearLine();
self.turnOffEditorMode();

if (!(self.bufferedCommand && self.bufferedCommand.length > 0) && empty) {
if (sawSIGINT) {
Expand All @@ -454,6 +462,11 @@ function REPLServer(prompt,
debug('line %j', cmd);
sawSIGINT = false;

if (self.editorMode) {
self.bufferedCommand += cmd + '\n';
return;
}

// leading whitespaces in template literals should not be trimmed.
if (self._inTemplateLiteral) {
self._inTemplateLiteral = false;
Expand Down Expand Up @@ -499,7 +512,8 @@ function REPLServer(prompt,

// If error was SyntaxError and not JSON.parse error
if (e) {
if (e instanceof Recoverable && !self.lineParser.shouldFail) {
if (e instanceof Recoverable && !self.lineParser.shouldFail &&
!sawCtrlD) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why sawCtrlD has to be checked here? When finish is executed, sawCtrlD should be false only, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thefourtheye sawCtrlD is set to true to indicate that current executed command is sourced from editor mode

// Start buffering data like that:
// {
// ... x: 1
Expand All @@ -515,6 +529,7 @@ function REPLServer(prompt,
// Clear buffer if no SyntaxErrors
self.lineParser.reset();
self.bufferedCommand = '';
sawCtrlD = false;

// If we got any output - print it (if no error)
if (!e &&
Expand Down Expand Up @@ -555,9 +570,55 @@ function REPLServer(prompt,
});

self.on('SIGCONT', function() {
self.displayPrompt(true);
if (self.editorMode) {
self.outputStream.write(`${self._initialPrompt}.editor\n`);
self.outputStream.write(
'// Entering editor mode (^D to finish, ^C to cancel)\n');
self.outputStream.write(`${self.bufferedCommand}\n`);
self.prompt(true);
} else {
self.displayPrompt(true);
}
});

// Wrap readline tty to enable editor mode
const ttyWrite = self._ttyWrite.bind(self);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like I am totally missing something here. Why _ttyWrite needs to be bound to self?

Copy link
Contributor Author

@princejwesley princejwesley Jun 21, 2016

Choose a reason for hiding this comment

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

@thefourtheye Without it, ttyWrite binds to global instance

> let obj = { member: "Hi", access() { return this.member } }
undefined
> obj.access()
'Hi'
> newAccess = obj.access
[Function: access]
> newAccess()
undefined
>

self._ttyWrite = (d, key) => {
if (!self.editorMode || !self.terminal) {
ttyWrite(d, key);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like an infinite recursive call to me. Please help me understand this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thefourtheye

self._ttyWrite and ttyWrite are different functions. ttyWrite has access to original self._ttyWrite and is bounded by self reference.

object.method is a nice shortcut for languages which supports single dispatch, so it is same as
method(object).

ttyWrite => global.ttyWrite => ttyWrite(global)

Copy link
Contributor

Choose a reason for hiding this comment

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

I simply overlooked the fact that the original function is retained. This clears up a lot of things in my mind. Thanks.

return;
}

// editor mode
if (key.ctrl && !key.shift) {
switch (key.name) {
case 'd': // End editor mode
self.turnOffEditorMode();
sawCtrlD = true;
ttyWrite(d, { name: 'return' });
break;
case 'n': // Override next history item
case 'p': // Override previous history item
Copy link
Contributor

Choose a reason for hiding this comment

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

are these keys the user can use while in editor mode? if so, there's no documentation for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@trevnorris Yes but its not a new feature and is not specific to editor mode. Its part of readline module

break;
default:
ttyWrite(d, key);
}
} else {
switch (key.name) {
case 'up': // Override previous history item
case 'down': // Override next history item
break;
case 'tab':
// prevent double tab behavior
self._previousKey = null;
ttyWrite(d, key);
break;
default:
ttyWrite(d, key);
}
}
};

self.displayPrompt();
}
inherits(REPLServer, Interface);
Expand Down Expand Up @@ -680,6 +741,12 @@ REPLServer.prototype.setPrompt = function setPrompt(prompt) {
REPLServer.super_.prototype.setPrompt.call(this, prompt);
};

REPLServer.prototype.turnOffEditorMode = function() {
this.editorMode = false;
this.setPrompt(this._initialPrompt);
};


// A stream to push an array into a REPL
// used in REPLServer.complete
function ArrayStream() {
Expand Down Expand Up @@ -987,6 +1054,39 @@ function complete(line, callback) {
}
}

function longestCommonPrefix(arr = []) {
const cnt = arr.length;
if (cnt === 0) return '';
if (cnt === 1) return arr[0];

const first = arr[0];
// complexity: O(m * n)
for (let m = 0; m < first.length; m++) {
const c = first[m];
for (let n = 1; n < cnt; n++) {
const entry = arr[n];
if (m >= entry.length || c !== entry[m]) {
return first.substring(0, m);
}
}
}
return first;
}

REPLServer.prototype.completeOnEditorMode = (callback) => (err, results) => {
if (err) return callback(err);

const [completions, completeOn = ''] = results;
const prefixLength = completeOn.length;

if (prefixLength === 0) return callback(null, [[], completeOn]);

const isNotEmpty = (v) => v.length > 0;
const trimCompleteOnPrefix = (v) => v.substring(prefixLength);
const data = completions.filter(isNotEmpty).map(trimCompleteOnPrefix);
Copy link
Contributor

Choose a reason for hiding this comment

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

completions.filter((s) => s) will also work here. I think this is okay.


callback(null, [[`${completeOn}${longestCommonPrefix(data)}`], completeOn]);
};

/**
* Used to parse and execute the Node REPL commands.
Expand Down Expand Up @@ -1189,6 +1289,17 @@ function defineDefaultCommands(repl) {
this.displayPrompt();
}
});

repl.defineCommand('editor', {
help: 'Entering editor mode (^D to finish, ^C to cancel)',
action() {
if (!this.terminal) return;
this.editorMode = true;
REPLServer.super_.prototype.setPrompt.call(this, '');
this.outputStream.write(
'// Entering editor mode (^D to finish, ^C to cancel)\n');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the comment is not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thefourtheye It helps user to remember ^D for finish/save and ^C for cancel the input!

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant the double slash at the beginning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

// makes more appealing to me! (My opinion)

> .editor
Entering editor mode (^D to finish, ^C to cancel)
Math.sqrt(3)

^D
1.7320508075688772
>

vs

> .editor
// Entering editor mode (^D to finish, ^C to cancel)
Math.sqrt(3)

^D
1.7320508075688772
>

Copy link
Member

Choose a reason for hiding this comment

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

I prefer the // as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ya, it looks nice. Lets have it then.

}
});
}

function regexpEscape(s) {
Expand Down
55 changes: 55 additions & 0 deletions test/parallel/test-repl-.editor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
'use strict';

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

// \u001b[1G - Moves the cursor to 1st column
// \u001b[0J - Clear screen
// \u001b[3G - Moves the cursor to 3rd column
const terminalCode = '\u001b[1G\u001b[0J> \u001b[3G';

function run(input, output, event) {
const stream = new common.ArrayStream();
let found = '';

stream.write = (msg) => found += msg.replace('\r', '');

const expected = `${terminalCode}.editor\n` +
'// Entering editor mode (^D to finish, ^C to cancel)\n' +
`${input}${output}\n${terminalCode}`;

const replServer = repl.start({
prompt: '> ',
terminal: true,
input: stream,
output: stream,
useColors: false
});

stream.emit('data', '.editor\n');
stream.emit('data', input);
replServer.write('', event);
replServer.close();
assert.strictEqual(found, expected);
}

const tests = [
{
input: '',
output: '\n(To exit, press ^C again or type .exit)',
event: {ctrl: true, name: 'c'}
},
{
input: 'var i = 1;',
output: '',
event: {ctrl: true, name: 'c'}
},
{
input: 'var i = 1;\ni + 3',
output: '\n4',
event: {ctrl: true, name: 'd'}
}
];

tests.forEach(({input, output, event}) => run(input, output, event));
22 changes: 22 additions & 0 deletions test/parallel/test-repl-tab-complete.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,3 +348,25 @@ testCustomCompleterAsyncMode.complete('a', common.mustCall((error, data) => {
'a'
]);
}));

// tab completion in editor mode
const editorStream = new common.ArrayStream();
const editor = repl.start({
stream: editorStream,
terminal: true,
useColors: false
});

editorStream.run(['.clear']);
editorStream.run(['.editor']);

editor.completer('co', common.mustCall((error, data) => {
assert.deepStrictEqual(data, [['con'], 'co']);
}));

editorStream.run(['.clear']);
editorStream.run(['.editor']);

editor.completer('var log = console.l', common.mustCall((error, data) => {
assert.deepStrictEqual(data, [['console.log'], 'console.l']);
}));