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
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: Add editor mode support
```js
> node
> .editor
// Entering editor mode (^D to finish, ^C to cancel)
function test() {
  console.log('tested!');
}

test();

// ^D
tested!
undefined
>
```
princejwesley committed Aug 3, 2016
commit 9eb96a14904430b1cadc0772177a5bf2c5118702
1 change: 1 addition & 0 deletions doc/api/repl.md
Original file line number Diff line number Diff line change
@@ -38,6 +38,7 @@ 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?


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

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

self._inTemplateLiteral = false;

@@ -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,
@@ -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) {
@@ -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;
@@ -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
@@ -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 &&
@@ -555,9 +570,50 @@ 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;
default:
ttyWrite(d, key);
}
}
};

self.displayPrompt();
}
inherits(REPLServer, Interface);
@@ -680,6 +736,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() {
@@ -987,6 +1049,39 @@ function complete(line, callback) {
}
}

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]);

function longestCommonPrefix(arr = []) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be declared outside of the enclosing function?

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;
}
};

/**
* Used to parse and execute the Node REPL commands.
@@ -1189,6 +1284,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) {
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(found === expected, `Expected: ${expected}, Found: ${found}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use assert.strictEqual instead of just assert here?

}

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
@@ -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']);
}));