Skip to content

feat: add support for persisting user preferences using a default configuration in the REPL #2559

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.

Sign up for GitHub

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

Open
Snehil-Shah wants to merge 6 commits into
base: develop
Choose a base branch
from
+210 −27
Open

feat: add support for persisting user preferences using a default configuration in the REPL #2559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
58eb85c
feat: add persistence using a default configuration
Snehil-Shah Jul 11, 2024
ad0cb06
fix: validate new themes
Snehil-Shah Jul 11, 2024
734560c
Merge branch 'stdlib-js:develop' into config
Snehil-Shah Jul 22, 2024
c1a4c1c
feat: allow resolving config from a `.js` export
Snehil-Shah Jul 22, 2024
1bc2c51
feat: explicitly prompt user before saving configuration
Snehil-Shah Jul 23, 2024
49b4672
refactor: replace keybinding with an input prompt
Snehil-Shah Jul 24, 2024
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
4 changes: 4 additions & 0 deletions lib/node_modules/@stdlib/repl/lib/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
var stdin = require( '@stdlib/streams/node/stdin' );
var stdout = require( '@stdlib/streams/node/stdout' );
var WELCOME = require( './welcome_text.js' );
var THEMES = require( './themes.js' );


// MAIN //
Expand Down Expand Up @@ -66,6 +67,9 @@ function defaults() {
// Welcome message:
'welcome': WELCOME,

// Syntax-highlighting themes:
'themes': THEMES,

// File path specifying where to save REPL command history:
'save': '',

Expand Down
190 changes: 169 additions & 21 deletions lib/node_modules/@stdlib/repl/lib/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ var EventEmitter = require( 'events' ).EventEmitter;
var readline = require( 'readline' );
var proc = require( 'process' );
var resolve = require( 'path' ).resolve;
var join = require( 'path' ).join;
var mkdir = require( 'fs' ).mkdirSync; // eslint-disable-line node/no-sync
var dirname = require( 'path' ).dirname;
var logger = require( 'debug' );
var inherit = require( '@stdlib/utils/inherit' );
var isString = require( '@stdlib/assert/is-string' ).isPrimitive;
Expand All @@ -34,7 +37,11 @@ var isPlainObject = require( '@stdlib/assert/is-plain-object' );
var isFunction = require( '@stdlib/assert/is-function' );
var isConfigurableProperty = require( '@stdlib/assert/is-configurable-property' );
var hasOwnProp = require( '@stdlib/assert/has-own-property' );
var objectKeys = require( '@stdlib/utils/keys' );
var isEmptyObject = require( '@stdlib/assert/is-empty-object' );
var deepEqual = require( '@stdlib/assert/deep-equal' );
var copy = require( '@stdlib/utils/copy' );
var pick = require( '@stdlib/utils/pick' );
var mergeFcn = require( '@stdlib/utils/merge' ).factory;
var setNonEnumerable = require( '@stdlib/utils/define-nonenumerable-property' );
var setNonEnumerableReadOnly = require( '@stdlib/utils/define-nonenumerable-read-only-property' );
var setReadOnly = require( '@stdlib/utils/define-read-only-property' );
Expand All @@ -43,7 +50,11 @@ var append = require( '@stdlib/utils/append' );
var format = require( '@stdlib/string/format' );
var Boolean = require( '@stdlib/boolean/ctor' );
var cwd = require( '@stdlib/process/cwd' );
var resolveParentPath = require( '@stdlib/fs/resolve-parent-path' ).sync;
var homeDir = require( '@stdlib/os/homedir' );
var dirExists = require( '@stdlib/fs/exists' ).sync;
var readFileSync = require( '@stdlib/fs/read-file' ).sync;
var writeFileSync = require( '@stdlib/fs/write-file' ).sync;
var RE_EOL = require( '@stdlib/regexp/eol' ).REGEXP;
var fifo = require( '@stdlib/utils/fifo' );
var nextTick = require( '@stdlib/utils/next-tick' );
Expand Down Expand Up @@ -75,6 +86,22 @@ var SETTINGS_VALIDATORS = require( './settings_validators.js' );
// VARIABLES //

var debug = logger( 'repl' );
var mergeOptions = mergeFcn({
'level': 2 // to merge individual settings and themes
});
Comment on lines +89 to +91
Copy link
Member Author

@Snehil-Shah Snehil-Shah Aug 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I am still skeptical if this is the best way to do this. For context, after resolving the options from the configuration file, I am merging it with the defaults(), till level 2. My main concern being, we might not always want to merge all things till level 2 (and things like the input and output stream properties might also get wrongly merged creating corrupted data), should we have a custom merger (similar to the validate function)?

Copy link
Member

@kgryte kgryte Aug 27, 2024

Choose a reason for hiding this comment

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

Would you be able to show in code what you are concerned about? In particular, can you provide example config which gets merged and creates "corrupted" opts? Would help me, as I am having trouble visualizing the potential issue.

Copy link
Member Author

@Snehil-Shah Snehil-Shah Aug 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

the defaults() also has opts.input which is a stream object (with many nested properties at diff levels), say someone makes a custom configuration, with an input option to a different object, they can potentially inject and corrupt the stream object at some levels because of the merge. We do have a list of PERSISTABLE_OPTIONS (L94), that we are using to allow saving only those specific properties that we want to persist. Maybe we can use it to also enforce that only the PERSISTABLE_OPTIONS can be loaded through a custom configuration. But again, this isn't future-proof, as it can be something else other than streams as well that we might later support.

Currently, in the validate function, we manually check if the given options (at REPL instantiation) has the prop or not (for a property, say settings), if yes, we merge or replace the default with the given property manually for each option. So, should we do something similar to merge 1) defaults, 2) configuration, and 3) REPL options into a final opts object?


// List of persistable options (note: keep in alphabetical order):
var PERSISTABLE_OPTIONS = [
'inputPrompt',
'outputPrompt',
'welcome',
'padding',
'themes',
'save',
'log',
'quiet',
'settings'
];


// MAIN //
Expand Down Expand Up @@ -129,12 +156,12 @@ var debug = logger( 'repl' );
* repl.close();
*/
function REPL( options ) {
var configPath;
var ostream;
var themes;
var config;
var opts;
var self;
var err;
var i;

if ( !( this instanceof REPL ) ) {
if ( arguments.length ) {
Expand All @@ -145,6 +172,9 @@ function REPL( options ) {
self = this;

opts = defaults();
configPath = this._resolveConfigurationPath();
config = this._getConfiguration( configPath );
opts = mergeOptions( opts, config );
if ( arguments.length ) {
err = validate( opts, options );
if ( err ) {
Expand Down Expand Up @@ -184,6 +214,12 @@ function REPL( options ) {
// Setup the output stream pipeline:
ostream.pipe( opts.output );

// Cache references to initial REPL configuration:
setNonEnumerableReadOnly( this, '_config', config );
setNonEnumerableReadOnly( this, '_configPath', configPath );
setNonEnumerableReadOnly( this, '_options', options );
setNonEnumerableReadOnly( this, '_opts', copy( opts ) );

// Cache references to the input and output streams:
setNonEnumerableReadOnly( this, '_istream', opts.input );
setNonEnumerableReadOnly( this, '_ostream', ostream );
Expand Down Expand Up @@ -248,8 +284,8 @@ function REPL( options ) {
// Initialize an internal flag indicating whether the REPL is currently busy with asynchronous processing:
setNonEnumerable( this, '_busy', false );

// Initialize an internal flag indicating whether we've received a `SIGINT` signal:
setNonEnumerable( this, '_SIGINT', false );
// Initialize an internal counter indicating number of processed `SIGINT` signals:
setNonEnumerable( this, '_SIGINT', 0 );

// Initialize an internal variable for caching the result of the last successfully evaluated command:
setNonEnumerable( this, '_ans', void 0 );
Expand Down Expand Up @@ -285,7 +321,7 @@ function REPL( options ) {
setNonEnumerableReadOnly( this, '_previewCompleter', new PreviewCompleter( this._rli, this._completer, this._ostream, this._settings.completionPreviews ) );

// Initialize a syntax-highlighter:
setNonEnumerableReadOnly( this, '_syntaxHighlighter', new SyntaxHighlighter( this, this._ostream, this._settings.syntaxHighlighting ) );
setNonEnumerableReadOnly( this, '_syntaxHighlighter', new SyntaxHighlighter( this, this._ostream, opts.themes, this._settings.theme, this._settings.syntaxHighlighting ) );

// Cache a reference to the private readline interface `ttyWrite` to allow calling the method when wanting default behavior:
setNonEnumerableReadOnly( this, '_ttyWrite', this._rli._ttyWrite );
Expand Down Expand Up @@ -315,16 +351,6 @@ function REPL( options ) {

// TODO: check whether to synchronously initialize a REPL log file

// Add any provided user-defined themes...
if ( opts.themes ) {
themes = objectKeys( opts.themes );
for ( i = 0; i < themes.length; i++ ) {
this.addTheme( themes[ i ], opts.themes[ themes[ i ] ] );
}
}
// Set the syntax highlighting theme...
this.settings( 'theme', opts.settings.theme );

// Initialize bracketed-paste:
if ( opts.settings.bracketedPaste !== void 0 ) {
this.settings( 'bracketedPaste', opts.settings.bracketedPaste );
Expand Down Expand Up @@ -400,7 +426,7 @@ function REPL( options ) {
* @param {string} line - line data
*/
function onLine( line ) {
self._SIGINT = false; // reset flag
self._SIGINT = 0; // reset flag
if ( self._closed === false ) {
self._multilineHandler.processLine( line );
}
Expand Down Expand Up @@ -465,15 +491,25 @@ function REPL( options ) {

// In the absence of any entered partial and/or unevaluated commands, determine whether we should close the REPL...
if ( self._cmd.length === 0 && isEmpty ) {
if ( self._SIGINT ) {
self._SIGINT = false;
if ( self._SIGINT === 2 ) {
self._SIGINT = 0;
self.close();
return;
}
if ( self._SIGINT === 1 ) {
if ( self._detectUnsavedChanges() ) {
self._SIGINT += 1;
self._rli.question( 'You have unsaved preferences. Do you want to save? [Y/N]: ', onAnswer );
return;
}
self._SIGINT = 0;
self.close();
return;
}
self._ostream.write( 'To exit, enter ^D or ^C again or enter quit().\n' );
self._SIGINT = true;
self._SIGINT += 1;
} else {
self._SIGINT = false;
self._SIGINT = 0;
}
// Reset the command queue:
self._queue.clear();
Expand All @@ -483,6 +519,28 @@ function REPL( options ) {

// Display a new prompt:
displayPrompt( self, false );

/**
* Callback invoked upon receiving an answer to a question.
*
* @private
* @param {string} answer - user answer
*/
function onAnswer( answer ) {
if ( answer === 'y' || answer === 'Y' ) {
self._saveConfiguration( config, options );
self._SIGINT = 0;
self._ostream.write( 'Preferences saved.\n' );
self.close();
return;
}
if ( answer === 'n' || answer === 'N' ) {
self._SIGINT = 0;
self.close();
return;
}
self._rli.question( 'You have unsaved preferences. Do you want to save? [Y/N]: ', onAnswer );
}
}

/**
Expand Down Expand Up @@ -538,6 +596,96 @@ setNonEnumerableReadOnly( REPL.prototype, '_prompt', function prompt() {
return inputPrompt( this._inputPrompt, this._count );
});

/**
* Resolves any existing REPL configuration path.
*
* @private
* @name _resolveConfigurationPath
* @memberof REPL.prototype
* @type {Function}
* @returns {string} configuration file path
*/
setNonEnumerableReadOnly( REPL.prototype, '_resolveConfigurationPath', function resolveConfigurationPath() {
var f;

f = resolveParentPath( '.stdlib_repl.json' );
if ( f ) {
return f;
}
return join( homeDir(), '.stdlib', 'repl.json' );
});

/**
* Reads from a configuration file path.
*
* @private
* @name _getConfiguration
* @memberof REPL.prototype
* @type {Function}
* @param {string} fpath - configuration file path
* @returns {Object} configuration object
*/
setNonEnumerableReadOnly( REPL.prototype, '_getConfiguration', function getConfiguration( fpath ) {
/* eslint-disable stdlib/no-dynamic-require, no-unused-vars */
var config;
try {
config = require( fpath );
if ( isPlainObject( config ) ) {
return config;
}
} catch ( e ) {
return {};
}
return {};
});

/**
* Saves preferences to a REPL configuration file.
*
* @private
* @name _saveConfiguration
* @memberof REPL.prototype
* @type {Function}
* @returns {void}
*/
setNonEnumerableReadOnly( REPL.prototype, '_saveConfiguration', function saveConfiguration() {
var newConfig;
var options;
var path;

options = pick( this._options, PERSISTABLE_OPTIONS );
newConfig = mergeOptions( this._config, options );
newConfig.settings = this._settings;
newConfig.themes = this._syntaxHighlighter.getAllThemesConfig();
path = dirname( this._configPath );
if ( !dirExists( path ) ) {
mkdir( path );
}
writeFileSync( this._configPath, JSON.stringify( newConfig, null, 2 ) );
});

/**
* Checks if there are any unsaved user preferences in the current REPL session.
*
* @private
* @name _detectUnsavedChanges
* @memberof REPL.prototype
* @type {Function}
* @returns {boolean} boolean indicating whether there are unsaved preferences in the REPL
*/
setNonEnumerableReadOnly( REPL.prototype, '_detectUnsavedChanges', function detectUnsavedChanges() {
if ( !isEmptyObject( this._options ) ) {
return true;
}
if ( !deepEqual( this._opts.settings, this._settings ) ) {
return true;
}
if ( !deepEqual( this._opts.themes, this._syntaxHighlighter.getAllThemesConfig() ) ) { // eslint-disable-line max-len
return true;
}
return false;
});

/**
* Returns the current line's prompt length.
*
Expand Down
23 changes: 18 additions & 5 deletions lib/node_modules/@stdlib/repl/lib/syntax_highlighter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ var objectKeys = require( '@stdlib/utils/keys' );
var omit = require( '@stdlib/utils/omit' );
var hasOwnProp = require( '@stdlib/assert/has-own-property' );
var tokenizer = require( './tokenizer.js' );
var THEMES = require( './themes.js' );
var ANSI = require( './ansi_colors.js' );


Expand Down Expand Up @@ -85,12 +84,14 @@ function removeDuplicateTokens( tokens ) {
* @constructor
* @param {REPL} repl - REPL instance
* @param {WritableStream} ostream - writable stream
* @param {Object} themes - table of initial syntax-highlighting themes
* @param {theme} theme - initial color theme
* @param {boolean} enabled - boolean indicating whether the syntax-highlighter should be initially enabled
* @returns {SyntaxHighlighter} syntax-highlighter instance
*/
function SyntaxHighlighter( repl, ostream, enabled ) {
function SyntaxHighlighter( repl, ostream, themes, theme, enabled ) {
if ( !( this instanceof SyntaxHighlighter ) ) {
return new SyntaxHighlighter( repl, ostream, enabled );
return new SyntaxHighlighter( repl, ostream, themes, theme, enabled );
}
debug( 'Creating a new syntax-highlighter' );

Expand All @@ -116,10 +117,10 @@ function SyntaxHighlighter( repl, ostream, enabled ) {
this._highlightedLine = '';

// Initialize an object storing all available themes:
this._themes = THEMES;
this._themes = themes;

// Initialize a variable storing the current theme:
this._theme = DEFAULT_THEME;
this._theme = theme;

return this;
}
Expand Down Expand Up @@ -216,6 +217,18 @@ setNonEnumerableReadOnly( SyntaxHighlighter.prototype, 'getThemeConfig', functio
return this._themes[ theme ];
});

/**
* Returns all color themes.
*
* @name getAllThemesConfig
* @memberof SyntaxHighlighter.prototype
* @type {Function}
* @returns {Object} themes object
*/
setNonEnumerableReadOnly( SyntaxHighlighter.prototype, 'getAllThemesConfig', function getAllThemesConfig() {
return this._themes;
});

/**
* Adds a new color theme.
*
Expand Down
Loading