From 0fb6ffb36c1a90093f4a582b37f92d6864709dbf Mon Sep 17 00:00:00 2001 From: Prince J Wesley Date: Mon, 15 Aug 2016 17:37:01 +0530 Subject: [PATCH 1/7] readline: key interval delay for \r & \n Emit two line events when there is a delay between CR('\r') and LF('\n') --- lib/readline.js | 24 ++++++++++++++---------- test/parallel/test-readline-interface.js | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/lib/readline.js b/lib/readline.js index f7591b7cc1663b..4464743feb791b 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -7,6 +7,7 @@ 'use strict'; const kHistorySize = 30; +const kCRLFDelayInMillis = 100; const util = require('util'); const debug = util.debuglog('readline'); @@ -39,7 +40,7 @@ function Interface(input, output, completer, terminal) { return self; } - this._sawReturn = false; + this._sawReturnAt = 0; this.isCompletionEnabled = true; this._previousKey = null; @@ -341,9 +342,10 @@ Interface.prototype._normalWrite = function(b) { return; } var string = this._decoder.write(b); - if (this._sawReturn) { + if (this._sawReturnAt && + Date.now() - this._sawReturnAt <= kCRLFDelayInMillis) { string = string.replace(/^\n/, ''); - this._sawReturn = false; + this._sawReturnAt = 0; } // Run test() on the new string chunk, not on the entire line buffer. @@ -354,7 +356,7 @@ Interface.prototype._normalWrite = function(b) { this._line_buffer = null; } if (newPartContainsEnding) { - this._sawReturn = string.endsWith('\r'); + this._sawReturnAt = string.endsWith('\r') ? Date.now() : 0; // got one or more newlines; process into "line" events var lines = string.split(lineEnding); @@ -842,20 +844,22 @@ Interface.prototype._ttyWrite = function(s, key) { /* No modifier keys used */ // \r bookkeeping is only relevant if a \n comes right after. - if (this._sawReturn && key.name !== 'enter') - this._sawReturn = false; + if (this._sawReturnAt && key.name !== 'enter') + this._sawReturnAt = 0; switch (key.name) { case 'return': // carriage return, i.e. \r - this._sawReturn = true; + this._sawReturnAt = Date.now(); this._line(); break; case 'enter': - if (this._sawReturn) - this._sawReturn = false; - else + // When key interval > kCRLFDelayInMillis + if (this._sawReturnAt === 0 || + Date.now() - this._sawReturnAt > kCRLFDelayInMillis) { this._line(); + } + this._sawReturnAt = 0; break; case 'backspace': diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 4a4b2f896961e8..3d66ff631e48e8 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -199,6 +199,27 @@ function isWarned(emitter) { assert.equal(callCount, expectedLines.length); rli.close(); + // Emit two line events where there is a delay + // between \r and \n + (() => { + const fi = new FakeInput(); + const rli = new readline.Interface({ + input: fi, + output: fi, + terminal: terminal + }); + let callCount = 0; + rli.on('line', function(line) { + callCount++; + }); + fi.emit('data', '\r'); + setTimeout(() => { + fi.emit('data', '\n'); + assert.equal(callCount, 2); + rli.close(); + }, 200); + })(); + // \t when there is no completer function should behave like an ordinary // character fi = new FakeInput(); From eb49af73f2f403992aa039603142f40ca2fe1132 Mon Sep 17 00:00:00 2001 From: Prince J Wesley Date: Mon, 15 Aug 2016 20:26:20 +0530 Subject: [PATCH 2/7] Braces for block scope --- test/parallel/test-readline-interface.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 3d66ff631e48e8..bc9f2356ecdc3a 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -201,7 +201,7 @@ function isWarned(emitter) { // Emit two line events where there is a delay // between \r and \n - (() => { + { const fi = new FakeInput(); const rli = new readline.Interface({ input: fi, @@ -218,7 +218,7 @@ function isWarned(emitter) { assert.equal(callCount, 2); rli.close(); }, 200); - })(); + } // \t when there is no completer function should behave like an ordinary // character From a13083d334e7e253a6b7406d0630f69b4fb76d7d Mon Sep 17 00:00:00 2001 From: Prince J Wesley Date: Tue, 16 Aug 2016 01:24:31 +0530 Subject: [PATCH 3/7] Configurable delay option --- doc/api/readline.md | 3 +++ lib/readline.js | 24 ++++++++++++++++++------ test/parallel/test-readline-interface.js | 15 +++++++++++---- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/doc/api/readline.md b/doc/api/readline.md index c01397d57108b6..e09ad317973db5 100644 --- a/doc/api/readline.md +++ b/doc/api/readline.md @@ -358,6 +358,9 @@ added: v0.1.98 only if `terminal` is set to `true` by the user or by an internal `output` check, otherwise the history caching mechanism is not initialized at all. * `prompt` - the prompt string to use. Default: `'> '` + * `crLfDelay` {number} If the delay between `\r` and `\n` exceeds + `crLfDelay`, both `\r` and `\n` will be treated as separate end-of-line + input. Default to `100` milliseconds. The `readline.createInterface()` method creates a new `readline.Interface` instance. diff --git a/lib/readline.js b/lib/readline.js index 4464743feb791b..97cdb2d501cae1 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -21,18 +21,20 @@ const isFullWidthCodePoint = internalReadline.isFullWidthCodePoint; const stripVTControlCharacters = internalReadline.stripVTControlCharacters; -exports.createInterface = function(input, output, completer, terminal) { +exports.createInterface = function(input, output, completer, terminal, + crLfDelay = kCRLFDelayInMillis) { var rl; if (arguments.length === 1) { rl = new Interface(input); } else { - rl = new Interface(input, output, completer, terminal); + rl = new Interface(input, output, completer, terminal, crLfDelay); } return rl; }; -function Interface(input, output, completer, terminal) { +function Interface(input, output, completer, terminal, + crLfDelay = kCRLFDelayInMillis) { if (!(this instanceof Interface)) { // call the constructor preserving original number of arguments const self = Object.create(Interface.prototype); @@ -57,6 +59,9 @@ function Interface(input, output, completer, terminal) { if (input.prompt !== undefined) { prompt = input.prompt; } + if (input.crLfDelay !== undefined) { + crLfDelay = input.crLfDelay; + } input = input.input; } @@ -74,6 +79,12 @@ function Interface(input, output, completer, terminal) { throw new TypeError('Argument "historySize" must be a positive number'); } + if (typeof crLfDelay !== 'number' || + isNaN(crLfDelay) || + crLfDelay <= 0) { + throw new TypeError('Argument "crLfDelay" must be a positive number > 0'); + } + // backwards compat; check the isTTY prop of the output stream // when `terminal` was not specified if (terminal === undefined && !(output === null || output === undefined)) { @@ -85,6 +96,7 @@ function Interface(input, output, completer, terminal) { this.output = output; this.input = input; this.historySize = historySize; + this.crLfDelay = crLfDelay; // Check arity, 2 - for async, 1 for sync if (typeof completer === 'function') { @@ -343,7 +355,7 @@ Interface.prototype._normalWrite = function(b) { } var string = this._decoder.write(b); if (this._sawReturnAt && - Date.now() - this._sawReturnAt <= kCRLFDelayInMillis) { + Date.now() - this._sawReturnAt <= this.crLfDelay) { string = string.replace(/^\n/, ''); this._sawReturnAt = 0; } @@ -854,9 +866,9 @@ Interface.prototype._ttyWrite = function(s, key) { break; case 'enter': - // When key interval > kCRLFDelayInMillis + // When key interval > crLfDelay if (this._sawReturnAt === 0 || - Date.now() - this._sawReturnAt > kCRLFDelayInMillis) { + Date.now() - this._sawReturnAt > this.crLfDelay) { this._line(); } this._sawReturnAt = 0; diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index bc9f2356ecdc3a..47270b3748d3be 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -46,6 +46,11 @@ function isWarned(emitter) { rli = new readline.Interface({ input: fi, output: fi, terminal: terminal}); assert.strictEqual(rli.historySize, 30); + // default crLfDelay is 100ms + fi = new FakeInput(); + rli = new readline.Interface({ input: fi, output: fi, terminal: terminal}); + assert.strictEqual(rli.crLfDelay, 100); + fi.emit('data', 'asdf\n'); assert.deepStrictEqual(rli.history, terminal ? ['asdf'] : undefined); rli.close(); @@ -199,14 +204,16 @@ function isWarned(emitter) { assert.equal(callCount, expectedLines.length); rli.close(); - // Emit two line events where there is a delay - // between \r and \n + // Emit two line events when the delay + // between \r and \n exceeds crLFDelay { const fi = new FakeInput(); + const delay = 200; const rli = new readline.Interface({ input: fi, output: fi, - terminal: terminal + terminal: terminal, + crLfDelay: delay }); let callCount = 0; rli.on('line', function(line) { @@ -217,7 +224,7 @@ function isWarned(emitter) { fi.emit('data', '\n'); assert.equal(callCount, 2); rli.close(); - }, 200); + }, delay * 2); } // \t when there is no completer function should behave like an ordinary From f5cf5316ab7af01ee1a544f2cc4ae23508e8075d Mon Sep 17 00:00:00 2001 From: Prince J Wesley Date: Tue, 16 Aug 2016 09:39:54 +0530 Subject: [PATCH 4/7] option name & alignment fix --- doc/api/readline.md | 4 ++-- lib/readline.js | 28 ++++++++++++------------ test/parallel/test-readline-interface.js | 8 +++---- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/doc/api/readline.md b/doc/api/readline.md index e09ad317973db5..fee8bcffb99d39 100644 --- a/doc/api/readline.md +++ b/doc/api/readline.md @@ -358,8 +358,8 @@ added: v0.1.98 only if `terminal` is set to `true` by the user or by an internal `output` check, otherwise the history caching mechanism is not initialized at all. * `prompt` - the prompt string to use. Default: `'> '` - * `crLfDelay` {number} If the delay between `\r` and `\n` exceeds - `crLfDelay`, both `\r` and `\n` will be treated as separate end-of-line + * `crlfDelay` {number} If the delay between `\r` and `\n` exceeds + `crlfDelay`, both `\r` and `\n` will be treated as separate end-of-line input. Default to `100` milliseconds. The `readline.createInterface()` method creates a new `readline.Interface` diff --git a/lib/readline.js b/lib/readline.js index 97cdb2d501cae1..8d8f3b90ac245a 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -7,7 +7,7 @@ 'use strict'; const kHistorySize = 30; -const kCRLFDelayInMillis = 100; +const kcrlfDelayInMillis = 100; const util = require('util'); const debug = util.debuglog('readline'); @@ -22,19 +22,19 @@ const stripVTControlCharacters = internalReadline.stripVTControlCharacters; exports.createInterface = function(input, output, completer, terminal, - crLfDelay = kCRLFDelayInMillis) { + crlfDelay = kcrlfDelayInMillis) { var rl; if (arguments.length === 1) { rl = new Interface(input); } else { - rl = new Interface(input, output, completer, terminal, crLfDelay); + rl = new Interface(input, output, completer, terminal, crlfDelay); } return rl; }; function Interface(input, output, completer, terminal, - crLfDelay = kCRLFDelayInMillis) { + crlfDelay = kcrlfDelayInMillis) { if (!(this instanceof Interface)) { // call the constructor preserving original number of arguments const self = Object.create(Interface.prototype); @@ -59,8 +59,8 @@ function Interface(input, output, completer, terminal, if (input.prompt !== undefined) { prompt = input.prompt; } - if (input.crLfDelay !== undefined) { - crLfDelay = input.crLfDelay; + if (input.crlfDelay !== undefined) { + crlfDelay = input.crlfDelay; } input = input.input; } @@ -79,10 +79,10 @@ function Interface(input, output, completer, terminal, throw new TypeError('Argument "historySize" must be a positive number'); } - if (typeof crLfDelay !== 'number' || - isNaN(crLfDelay) || - crLfDelay <= 0) { - throw new TypeError('Argument "crLfDelay" must be a positive number > 0'); + if (typeof crlfDelay !== 'number' || + isNaN(crlfDelay) || + crlfDelay <= 0) { + throw new TypeError('Argument "crlfDelay" must be a positive number > 0'); } // backwards compat; check the isTTY prop of the output stream @@ -96,7 +96,7 @@ function Interface(input, output, completer, terminal, this.output = output; this.input = input; this.historySize = historySize; - this.crLfDelay = crLfDelay; + this.crlfDelay = crlfDelay; // Check arity, 2 - for async, 1 for sync if (typeof completer === 'function') { @@ -355,7 +355,7 @@ Interface.prototype._normalWrite = function(b) { } var string = this._decoder.write(b); if (this._sawReturnAt && - Date.now() - this._sawReturnAt <= this.crLfDelay) { + Date.now() - this._sawReturnAt <= this.crlfDelay) { string = string.replace(/^\n/, ''); this._sawReturnAt = 0; } @@ -866,9 +866,9 @@ Interface.prototype._ttyWrite = function(s, key) { break; case 'enter': - // When key interval > crLfDelay + // When key interval > crlfDelay if (this._sawReturnAt === 0 || - Date.now() - this._sawReturnAt > this.crLfDelay) { + Date.now() - this._sawReturnAt > this.crlfDelay) { this._line(); } this._sawReturnAt = 0; diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 47270b3748d3be..7958a955fbeb3c 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -46,10 +46,10 @@ function isWarned(emitter) { rli = new readline.Interface({ input: fi, output: fi, terminal: terminal}); assert.strictEqual(rli.historySize, 30); - // default crLfDelay is 100ms + // default crlfDelay is 100ms fi = new FakeInput(); rli = new readline.Interface({ input: fi, output: fi, terminal: terminal}); - assert.strictEqual(rli.crLfDelay, 100); + assert.strictEqual(rli.crlfDelay, 100); fi.emit('data', 'asdf\n'); assert.deepStrictEqual(rli.history, terminal ? ['asdf'] : undefined); @@ -205,7 +205,7 @@ function isWarned(emitter) { rli.close(); // Emit two line events when the delay - // between \r and \n exceeds crLFDelay + // between \r and \n exceeds crlfDelay { const fi = new FakeInput(); const delay = 200; @@ -213,7 +213,7 @@ function isWarned(emitter) { input: fi, output: fi, terminal: terminal, - crLfDelay: delay + crlfDelay: delay }); let callCount = 0; rli.on('line', function(line) { From 515627c703c225d24550cf2489c34e1da539efdf Mon Sep 17 00:00:00 2001 From: Prince J Wesley Date: Tue, 16 Aug 2016 23:10:09 +0530 Subject: [PATCH 5/7] Coerce to Min/max range --- doc/api/readline.md | 5 +++-- lib/readline.js | 25 +++++++++--------------- test/parallel/test-readline-interface.js | 23 +++++++++++++++++----- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/doc/api/readline.md b/doc/api/readline.md index fee8bcffb99d39..6a2d8ebbd5b9e1 100644 --- a/doc/api/readline.md +++ b/doc/api/readline.md @@ -359,8 +359,9 @@ added: v0.1.98 check, otherwise the history caching mechanism is not initialized at all. * `prompt` - the prompt string to use. Default: `'> '` * `crlfDelay` {number} If the delay between `\r` and `\n` exceeds - `crlfDelay`, both `\r` and `\n` will be treated as separate end-of-line - input. Default to `100` milliseconds. + `crlfDelay` milliseconds, both `\r` and `\n` will be treated as separate + end-of-line input. Default to `100` milliseconds. + `crlfDelay` will be coerced to `[100, 2000]` range. The `readline.createInterface()` method creates a new `readline.Interface` instance. diff --git a/lib/readline.js b/lib/readline.js index 8d8f3b90ac245a..be21f4f7d07b9e 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -7,7 +7,8 @@ 'use strict'; const kHistorySize = 30; -const kcrlfDelayInMillis = 100; +const kMincrlfDelay = 100; +const kMaxcrlfDelay = 2000; const util = require('util'); const debug = util.debuglog('readline'); @@ -21,20 +22,18 @@ const isFullWidthCodePoint = internalReadline.isFullWidthCodePoint; const stripVTControlCharacters = internalReadline.stripVTControlCharacters; -exports.createInterface = function(input, output, completer, terminal, - crlfDelay = kcrlfDelayInMillis) { +exports.createInterface = function(input, output, completer, terminal) { var rl; if (arguments.length === 1) { rl = new Interface(input); } else { - rl = new Interface(input, output, completer, terminal, crlfDelay); + rl = new Interface(input, output, completer, terminal); } return rl; }; -function Interface(input, output, completer, terminal, - crlfDelay = kcrlfDelayInMillis) { +function Interface(input, output, completer, terminal) { if (!(this instanceof Interface)) { // call the constructor preserving original number of arguments const self = Object.create(Interface.prototype); @@ -48,6 +47,7 @@ function Interface(input, output, completer, terminal, EventEmitter.call(this); var historySize; + let crlfDelay; let prompt = '> '; if (arguments.length === 1) { @@ -59,9 +59,7 @@ function Interface(input, output, completer, terminal, if (input.prompt !== undefined) { prompt = input.prompt; } - if (input.crlfDelay !== undefined) { - crlfDelay = input.crlfDelay; - } + crlfDelay = input.crlfDelay; input = input.input; } @@ -79,12 +77,6 @@ function Interface(input, output, completer, terminal, throw new TypeError('Argument "historySize" must be a positive number'); } - if (typeof crlfDelay !== 'number' || - isNaN(crlfDelay) || - crlfDelay <= 0) { - throw new TypeError('Argument "crlfDelay" must be a positive number > 0'); - } - // backwards compat; check the isTTY prop of the output stream // when `terminal` was not specified if (terminal === undefined && !(output === null || output === undefined)) { @@ -96,7 +88,8 @@ function Interface(input, output, completer, terminal, this.output = output; this.input = input; this.historySize = historySize; - this.crlfDelay = crlfDelay; + this.crlfDelay = Math.max(kMincrlfDelay, + Math.min(kMaxcrlfDelay, crlfDelay >>> 0)); // Check arity, 2 - for async, 1 for sync if (typeof completer === 'function') { diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 7958a955fbeb3c..5df4cad6c69a9d 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -26,6 +26,24 @@ function isWarned(emitter) { return false; } +{ + // Default crlfDelay is 100ms + const fi = new FakeInput(); + let rli = new readline.Interface({ input: fi, output: fi }); + assert.strictEqual(rli.crlfDelay, 100); + rli.close(); + + // Minimum crlfDelay is 100ms + rli = new readline.Interface({ input: fi, output: fi, crlfDelay: 0}); + assert.strictEqual(rli.crlfDelay, 100); + rli.close(); + + // Maximum crlfDelay is 2000ms + rli = new readline.Interface({ input: fi, output: fi, crlfDelay: 1 << 30}); + assert.strictEqual(rli.crlfDelay, 2000); + rli.close(); +} + [ true, false ].forEach(function(terminal) { var fi; var rli; @@ -46,11 +64,6 @@ function isWarned(emitter) { rli = new readline.Interface({ input: fi, output: fi, terminal: terminal}); assert.strictEqual(rli.historySize, 30); - // default crlfDelay is 100ms - fi = new FakeInput(); - rli = new readline.Interface({ input: fi, output: fi, terminal: terminal}); - assert.strictEqual(rli.crlfDelay, 100); - fi.emit('data', 'asdf\n'); assert.deepStrictEqual(rli.history, terminal ? ['asdf'] : undefined); rli.close(); From 3b1b84e67aefbbe870628d5a0d550e204d4897d2 Mon Sep 17 00:00:00 2001 From: Prince J Wesley Date: Sun, 21 Aug 2016 13:53:28 +0530 Subject: [PATCH 6/7] scoped tests --- test/parallel/test-readline-interface.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 5df4cad6c69a9d..94948aed0fa2dd 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -29,17 +29,27 @@ function isWarned(emitter) { { // Default crlfDelay is 100ms const fi = new FakeInput(); - let rli = new readline.Interface({ input: fi, output: fi }); + const rli = new readline.Interface({ input: fi, output: fi }); assert.strictEqual(rli.crlfDelay, 100); rli.close(); +} +{ // Minimum crlfDelay is 100ms - rli = new readline.Interface({ input: fi, output: fi, crlfDelay: 0}); + const fi = new FakeInput(); + const rli = new readline.Interface({ input: fi, output: fi, crlfDelay: 0}); assert.strictEqual(rli.crlfDelay, 100); rli.close(); +} +{ // Maximum crlfDelay is 2000ms - rli = new readline.Interface({ input: fi, output: fi, crlfDelay: 1 << 30}); + const fi = new FakeInput(); + const rli = new readline.Interface({ + input: fi, + output: fi, + crlfDelay: 1 << 30 + }); assert.strictEqual(rli.crlfDelay, 2000); rli.close(); } From 9ed8f558fc1d890dcc3518d13bc250890e3debea Mon Sep 17 00:00:00 2001 From: Prince J Wesley Date: Wed, 24 Aug 2016 08:04:41 +0530 Subject: [PATCH 7/7] wrap function with mustCall --- test/parallel/test-readline-interface.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 94948aed0fa2dd..08dbdd488265ee 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -243,11 +243,11 @@ function isWarned(emitter) { callCount++; }); fi.emit('data', '\r'); - setTimeout(() => { + setTimeout(common.mustCall(() => { fi.emit('data', '\n'); assert.equal(callCount, 2); rli.close(); - }, delay * 2); + }), delay * 2); } // \t when there is no completer function should behave like an ordinary