From da336e475c7239b10faf31bb90cc9eda86c743f6 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Mon, 3 Aug 2015 13:01:31 -0700 Subject: [PATCH 1/5] repl: fix persistent history size limiting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since slice does not mutate the array, the history was never truncated. PR-URL: https://github.com/nodejs/io.js/pull/2224 Reviewed-By: Michaël Zasso Reviewed-By: Chris Dickinson Reviewed-By: Roman Reiss --- lib/internal/repl.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/internal/repl.js b/lib/internal/repl.js index 902e81f495573c..27252d3555b0d1 100644 --- a/lib/internal/repl.js +++ b/lib/internal/repl.js @@ -91,7 +91,9 @@ function setupHistory(repl, historyPath, ready) { if (!Array.isArray(repl.history)) { throw new Error('Expected array, got ' + typeof repl.history); } - repl.history.slice(-repl.historySize); + + repl.history = repl.history.slice(-repl.historySize); + } catch (err) { return ready( new Error(`Could not parse history data in ${historyPath}.`)); From 39eb22a4814c0fdaaa8d6d5beee2334f854ccee7 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Mon, 3 Aug 2015 13:10:19 -0700 Subject: [PATCH 2/5] repl: persist history in plain text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Persists the REPL history in plain text using the new NODE_REPL_HISTORY environment variable. Deprecates NODE_REPL_HISTORY_FILE. The REPL will notify the user and automatically convert the history to the new format if files are specified. PR-URL: https://github.com/nodejs/io.js/pull/2224 Reviewed-By: Michaël Zasso Reviewed-By: Chris Dickinson Reviewed-By: Roman Reiss --- lib/internal/repl.js | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/internal/repl.js b/lib/internal/repl.js index 27252d3555b0d1..a6a7cedfb24949 100644 --- a/lib/internal/repl.js +++ b/lib/internal/repl.js @@ -3,6 +3,7 @@ const Interface = require('readline').Interface; const REPL = require('repl'); const path = require('path'); +const os = require('os'); module.exports = Object.create(REPL); module.exports.createInternalRepl = createRepl; @@ -51,14 +52,15 @@ function createRepl(env, cb) { } const repl = REPL.start(opts); - if (opts.terminal && env.NODE_REPL_HISTORY_FILE) { - return setupHistory(repl, env.NODE_REPL_HISTORY_FILE, cb); + if (opts.terminal && env.NODE_REPL_HISTORY !== '') { + return setupHistory(repl, env.NODE_REPL_HISTORY, + env.NODE_REPL_HISTORY_FILE, cb); } repl._historyPrev = _replHistoryMessage; cb(null, repl); } -function setupHistory(repl, historyPath, ready) { +function setupHistory(repl, historyPath, oldHistoryPath, ready) { const fs = require('fs'); var timer = null; var writing = false; @@ -86,8 +88,16 @@ function setupHistory(repl, historyPath, ready) { } if (data) { + repl.history = data.split(/[\n\r]+/).slice(-repl.historySize); + } else if (oldHistoryPath) { + // Grab data from the older pre-v3.0 JSON NODE_REPL_HISTORY_FILE format. + repl._writeToOutput( + '\nConverting old JSON repl history to line-separated history.\n' + + `The new repl history file can be found at ${historyPath}.\n`); + repl._refreshLine(); + try { - repl.history = JSON.parse(data); + repl.history = JSON.parse(fs.readFileSync(oldHistoryPath, 'utf8')); if (!Array.isArray(repl.history)) { throw new Error('Expected array, got ' + typeof repl.history); } @@ -96,7 +106,7 @@ function setupHistory(repl, historyPath, ready) { } catch (err) { return ready( - new Error(`Could not parse history data in ${historyPath}.`)); + new Error(`Could not parse history data in ${oldHistoryPath}.`)); } } @@ -136,7 +146,7 @@ function setupHistory(repl, historyPath, ready) { return; } writing = true; - const historyData = JSON.stringify(repl.history, null, 2); + const historyData = repl.history.join(os.EOL); fs.write(repl._historyHandle, historyData, 0, 'utf8', onwritten); } @@ -159,7 +169,7 @@ function _replHistoryMessage() { if (this.history.length === 0) { this._writeToOutput( '\nPersistent history support disabled. ' + - 'Set the NODE_REPL_HISTORY_FILE environment variable to ' + + 'Set the NODE_REPL_HISTORY environment\nvariable to ' + 'a valid, user-writable path to enable.\n' ); this._refreshLine(); From 7776ccba7c9a95980dfa9db9b7bcfe1563e94f91 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Mon, 3 Aug 2015 23:24:03 -0700 Subject: [PATCH 3/5] repl: default persistence to ~/.node_repl_history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes the REPL persistently save history by default to ~/.node_repl_history when in terminal mode. This can be disabled by setting NODE_REPL_HISTORY="". PR-URL: https://github.com/nodejs/io.js/pull/2224 Reviewed-By: Michaël Zasso Reviewed-By: Chris Dickinson Reviewed-By: Roman Reiss --- lib/internal/repl.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/internal/repl.js b/lib/internal/repl.js index a6a7cedfb24949..ea6827f60dcfe3 100644 --- a/lib/internal/repl.js +++ b/lib/internal/repl.js @@ -3,7 +3,9 @@ const Interface = require('readline').Interface; const REPL = require('repl'); const path = require('path'); +const fs = require('fs'); const os = require('os'); +const debug = require('util').debuglog('repl'); module.exports = Object.create(REPL); module.exports.createInternalRepl = createRepl; @@ -61,7 +63,20 @@ function createRepl(env, cb) { } function setupHistory(repl, historyPath, oldHistoryPath, ready) { - const fs = require('fs'); + if (!historyPath) { + try { + historyPath = path.join(os.homedir(), '.node_repl_history'); + } catch (err) { + repl._writeToOutput('\nError: Could not get the home directory.\n' + + 'REPL session history will not be persisted.\n'); + repl._refreshLine(); + + debug(err.stack); + repl._historyPrev = _replHistoryMessage; + return ready(null, repl); + } + } + var timer = null; var writing = false; var pending = false; From 92880677167f48132cae4925dbf96b1e2942f5a0 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Sat, 1 Aug 2015 22:38:28 -0700 Subject: [PATCH 4/5] test: add tests for persistent repl history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/io.js/pull/2224 Reviewed-By: Michaël Zasso Reviewed-By: Chris Dickinson Reviewed-By: Roman Reiss --- .eslintrc | 2 + lib/internal/repl.js | 8 +- test/fixtures/.node_repl_history | 2 + test/fixtures/old-repl-history-file.json | 4 + .../test-repl-persistent-history.js | 193 ++++++++++++++++++ 5 files changed, 207 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/.node_repl_history create mode 100644 test/fixtures/old-repl-history-file.json create mode 100644 test/sequential/test-repl-persistent-history.js diff --git a/.eslintrc b/.eslintrc index c90fa5e57ad13e..532ac8b50cf07a 100644 --- a/.eslintrc +++ b/.eslintrc @@ -10,6 +10,8 @@ ecmaFeatures: generators: true forOf: true objectLiteralShorthandProperties: true + objectLiteralShorthandMethods: true + classes: true rules: # Possible Errors diff --git a/lib/internal/repl.js b/lib/internal/repl.js index ea6827f60dcfe3..d923dc8720c6ea 100644 --- a/lib/internal/repl.js +++ b/lib/internal/repl.js @@ -20,8 +20,12 @@ function replStart() { return REPL.start.apply(REPL, arguments); } -function createRepl(env, cb) { - const opts = { +function createRepl(env, opts, cb) { + if (typeof opts === 'function') { + cb = opts; + opts = null; + } + opts = opts || { ignoreUndefined: false, terminal: process.stdout.isTTY, useGlobal: true diff --git a/test/fixtures/.node_repl_history b/test/fixtures/.node_repl_history new file mode 100644 index 00000000000000..31ad6d3dc21938 --- /dev/null +++ b/test/fixtures/.node_repl_history @@ -0,0 +1,2 @@ +'you look fabulous today' +'Stay Fresh~' diff --git a/test/fixtures/old-repl-history-file.json b/test/fixtures/old-repl-history-file.json new file mode 100644 index 00000000000000..963d93ddf379c0 --- /dev/null +++ b/test/fixtures/old-repl-history-file.json @@ -0,0 +1,4 @@ +[ + "'=^.^='", + "'hello world'" +] diff --git a/test/sequential/test-repl-persistent-history.js b/test/sequential/test-repl-persistent-history.js new file mode 100644 index 00000000000000..8d550f6c1d7124 --- /dev/null +++ b/test/sequential/test-repl-persistent-history.js @@ -0,0 +1,193 @@ +'use strict'; + +// Flags: --expose-internals + +const common = require('../common'); +const stream = require('stream'); +const REPL = require('internal/repl'); +const assert = require('assert'); +const fs = require('fs'); +const util = require('util'); +const path = require('path'); +const os = require('os'); + +common.refreshTmpDir(); + +// Mock os.homedir() +os.homedir = function() { + return common.tmpDir; +}; + +// Create an input stream specialized for testing an array of actions +class ActionStream extends stream.Stream { + run(data) { + const _iter = data[Symbol.iterator](); + const self = this; + + function doAction() { + const next = _iter.next(); + if (next.done) { + // Close the repl. Note that it must have a clean prompt to do so. + setImmediate(function() { + self.emit('keypress', '', { ctrl: true, name: 'd' }); + }); + return; + } + const action = next.value; + + if (typeof action === 'object') { + self.emit('keypress', '', action); + } else { + self.emit('data', action + '\n'); + } + setImmediate(doAction); + } + setImmediate(doAction); + } + resume() {} + pause() {} +} +ActionStream.prototype.readable = true; + + +// Mock keys +const UP = { name: 'up' }; +const ENTER = { name: 'enter' }; +const CLEAR = { ctrl: true, name: 'u' }; +// Common message bits +const prompt = '> '; +const replDisabled = '\nPersistent history support disabled. Set the ' + + 'NODE_REPL_HISTORY environment\nvariable to a valid, ' + + 'user-writable path to enable.\n'; +const convertMsg = '\nConverting old JSON repl history to line-separated ' + + 'history.\nThe new repl history file can be found at ' + + path.join(common.tmpDir, '.node_repl_history') + '.\n'; +const homedirErr = '\nError: Could not get the home directory.\n' + + 'REPL session history will not be persisted.\n'; +// File paths +const fixtures = path.join(common.testDir, 'fixtures'); +const historyFixturePath = path.join(fixtures, '.node_repl_history'); +const historyPath = path.join(common.tmpDir, '.fixture_copy_repl_history'); +const oldHistoryPath = path.join(fixtures, 'old-repl-history-file.json'); + + +const tests = [{ + env: { NODE_REPL_HISTORY: '' }, + test: [UP], + expected: [prompt, replDisabled, prompt] +}, +{ + env: { NODE_REPL_HISTORY: '', + NODE_REPL_HISTORY_FILE: oldHistoryPath }, + test: [UP], + expected: [prompt, replDisabled, prompt] +}, +{ + env: { NODE_REPL_HISTORY: historyPath }, + test: [UP, CLEAR], + expected: [prompt, prompt + '\'you look fabulous today\'', prompt] +}, +{ + env: { NODE_REPL_HISTORY: historyPath, + NODE_REPL_HISTORY_FILE: oldHistoryPath }, + test: [UP, CLEAR], + expected: [prompt, prompt + '\'you look fabulous today\'', prompt] +}, +{ + env: { NODE_REPL_HISTORY: historyPath, + NODE_REPL_HISTORY_FILE: '' }, + test: [UP, CLEAR], + expected: [prompt, prompt + '\'you look fabulous today\'', prompt] +}, +{ + env: {}, + test: [UP], + expected: [prompt] +}, +{ + env: { NODE_REPL_HISTORY_FILE: oldHistoryPath }, + test: [UP, CLEAR, '\'42\'', ENTER/*, function(cb) { + // XXX(Fishrock123) Allow the REPL to save to disk. + // There isn't a way to do this programmatically right now. + setTimeout(cb, 50); + }*/], + expected: [prompt, convertMsg, prompt, prompt + '\'=^.^=\'', prompt, '\'', + '4', '2', '\'', '\'42\'\n', prompt, prompt], + after: function ensureHistoryFixture() { + // XXX(Fishrock123) Make sure nothing weird happened to our fixture + // or it's temporary copy. + // Sometimes this test used to erase the fixture and I'm not sure why. + const history = fs.readFileSync(historyFixturePath, 'utf8'); + assert.strictEqual(history, + '\'you look fabulous today\'\n\'Stay Fresh~\'\n'); + const historyCopy = fs.readFileSync(historyPath, 'utf8'); + assert.strictEqual(historyCopy, '\'you look fabulous today\'' + os.EOL + + '\'Stay Fresh~\'' + os.EOL); + } +}, +{ + env: {}, + test: [UP, UP, ENTER], + expected: [prompt, prompt + '\'42\'', prompt + '\'=^.^=\'', '\'=^.^=\'\n', + prompt] +}, +{ // Make sure this is always the last test, since we change os.homedir() + before: function mockHomedirFailure() { + // Mock os.homedir() failure + os.homedir = function() { + throw new Error('os.homedir() failure'); + }; + }, + env: {}, + test: [UP], + expected: [prompt, homedirErr, prompt, replDisabled, prompt] +}]; + + +// Copy our fixture to the tmp directory +fs.createReadStream(historyFixturePath) + .pipe(fs.createWriteStream(historyPath)).on('unpipe', runTest); + +function runTest() { + const opts = tests.shift(); + if (!opts) return; // All done + + const env = opts.env; + const test = opts.test; + const expected = opts.expected; + const after = opts.after; + const before = opts.before; + + if (before) before(); + + REPL.createInternalRepl(env, { + input: new ActionStream(), + output: new stream.Writable({ + write(chunk, _, next) { + const output = chunk.toString(); + + // Ignore escapes and blank lines + if (output.charCodeAt(0) === 27 || /^[\r\n]+$/.test(output)) + return next(); + + assert.strictEqual(output, expected.shift()); + next(); + } + }), + prompt: prompt, + useColors: false, + terminal: true + }, function(err, repl) { + if (err) throw err; + + if (after) repl.on('close', after); + + repl.on('close', function() { + // Ensure everything that we expected was output + assert.strictEqual(expected.length, 0); + setImmediate(runTest); + }); + + repl.inputStream.run(test); + }); +} From 3acf17d254cdef5cb42644f002ce3a1ede2c80f9 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Mon, 3 Aug 2015 23:33:48 -0700 Subject: [PATCH 5/5] doc: document repl persistent history changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/io.js/pull/2224 Reviewed-By: Michaël Zasso Reviewed-By: Chris Dickinson Reviewed-By: Roman Reiss --- doc/api/repl.markdown | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/doc/api/repl.markdown b/doc/api/repl.markdown index 551a70b2795424..8c992b3c9a84d2 100644 --- a/doc/api/repl.markdown +++ b/doc/api/repl.markdown @@ -29,23 +29,38 @@ For example, you could add this to your bashrc file: alias iojs="env NODE_NO_READLINE=1 rlwrap iojs" +### Persistent History + +By default, the REPL will persist history between `iojs` REPL sessions by saving +to a `.node_repl_history` file in the user's home directory. This can be +disabled by setting the environment variable `NODE_REPL_HISTORY=""`. + +Previously in io.js v2.x, REPL history was controlled by using a +`NODE_REPL_HISTORY_FILE` environment variable, and the history was saved in JSON +format. This variable has now been deprecated, and your REPL history will +automatically be converted to using plain text. The new file will be saved to +either your home directory, or a directory defined by the `NODE_REPL_HISTORY` +variable, as documented below. + +### Environment Variable Options + The built-in repl (invoked by running `iojs` or `iojs -i`) may be controlled via the following environment variables: - - `NODE_REPL_HISTORY_FILE` - if given, must be a path to a user-writable, - user-readable file. When a valid path is given, persistent history support - is enabled: REPL history will persist across `iojs` repl sessions. - - `NODE_REPL_HISTORY_SIZE` - defaults to `1000`. In conjunction with - `NODE_REPL_HISTORY_FILE`, controls how many lines of history will be - persisted. Must be a positive number. + - `NODE_REPL_HISTORY` - When a valid path is given, persistent REPL history + will be saved to the specified file rather than `.node_repl_history` in the + user's home directory. Setting this value to `""` will disable persistent + REPL history. + - `NODE_REPL_HISTORY_SIZE` - defaults to `1000`. Controls how many lines of + history will be persisted if history is available. Must be a positive number. - `NODE_REPL_MODE` - may be any of `sloppy`, `strict`, or `magic`. Defaults to `magic`, which will automatically run "strict mode only" statements in strict mode. ## repl.start(options) -Returns and starts a `REPLServer` instance, that inherits from -[Readline Interface][]. Accepts an "options" Object that takes +Returns and starts a `REPLServer` instance, that inherits from +[Readline Interface][]. Accepts an "options" Object that takes the following values: - `prompt` - the prompt and `stream` for all I/O. Defaults to `> `.