Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: remove SIGWINCH listener upon closing the REPL #2205

Merged
merged 1 commit into from
Apr 26, 2024
Merged
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
1 change: 1 addition & 0 deletions lib/node_modules/@stdlib/repl/lib/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@
setNonEnumerableReadOnly( this, '_ttyWrite', this._rli._ttyWrite );

// Overwrite the private `ttyWrite` method to allow processing input before a "keypress" event is triggered:
this._rli._ttyWrite = beforeKeypress; // WARNING: overwriting a private property

Check warning on line 277 in lib/node_modules/@stdlib/repl/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files

Unexpected 'warning' comment: 'WARNING: overwriting a private property'

// Add event listeners:
this._rli.on( 'close', onClose );
Expand All @@ -294,9 +294,9 @@
// Write a welcome message:
this._wstream.write( opts.welcome );

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

Check warning on line 297 in lib/node_modules/@stdlib/repl/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files

Unexpected 'todo' comment: 'TODO: check whether to synchronously...'

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

Check warning on line 299 in lib/node_modules/@stdlib/repl/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files

Unexpected 'todo' comment: 'TODO: check whether to synchronously...'

// Check whether to load and execute a JavaScript file (e.g., prior REPL history) upon startup...
if ( opts.load ) {
Expand Down Expand Up @@ -370,6 +370,7 @@

debug( 'Readline interface closed.' );
self._istream.removeListener( 'keypress', onKeypress );
proc.removeListener( 'SIGWINCH', onSIGWINCH );

debug( 'Exiting REPL...' );
self.emit( 'exit' );
Expand Down Expand Up @@ -444,9 +445,9 @@
// Update the internal command history buffer: [..., <id>, <cmd>, <success>, ...]
self._history.push( self._count, cmd, success );

// TODO: if successful and if necessary, (asynchronously?) write the command to a history file (question: do we only want to write successful commands to the history file? maybe we need to option for limiting to successful commands?)

Check warning on line 448 in lib/node_modules/@stdlib/repl/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files

Unexpected 'todo' comment: 'TODO: if successful and if necessary,...'

// TODO: if necessary, (asynchronously?) write the command and result to a log file (JSON serialization?)

Check warning on line 450 in lib/node_modules/@stdlib/repl/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files

Unexpected 'todo' comment: 'TODO: if necessary, (asynchronously?)...'
}
}

Expand Down Expand Up @@ -670,7 +671,7 @@

// Before creating a new execution context in a non-sandboxed environment, remove current workspace variables in order to allow garbage collection and avoid memory leaks (e.g., variables/functions declared during a REPL session which might remain bound to the environment `global` after clearing a REPL):
if ( this._sandbox === false ) {
// WARNING: in a non-sandboxed environment, if a global variable is externally introduced during a REPL session (i.e., introduced via a mechanism outside of the REPL environment), we will delete that global variable, which means the following logic may introduce unintended side-effects for this particular edge case (e.g., application code may expect the presence of the subsequently deleted global variable). While not ideal, (a) user applications should not be introducing globals to begin with and (b) the probability of a user running a REPL session, a user clearing that REPL session, AND a global variable being introduced between starting a REPL and clearing the REPL should be negligible.

Check warning on line 674 in lib/node_modules/@stdlib/repl/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files

Unexpected 'warning' comment: 'WARNING: in a non-sandboxed environment,...'
tmp = this._context.vars();
for ( i = 0; i < tmp.length; i++ ) {
if ( isConfigurableProperty( this._context, tmp[ i ] ) ) {
Expand Down Expand Up @@ -1063,9 +1064,9 @@
// Clear the command queue:
this._queue.clear();

// TODO: ensure REPL history is saved (flushed) to file before closing the REPL (see https://github.com/nodejs/node/blob/b21e7c7bcf23a2715951e4cd96180e4dbf1dcd4d/lib/repl.js#L805)

Check warning on line 1067 in lib/node_modules/@stdlib/repl/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files

Unexpected 'todo' comment: 'TODO: ensure REPL history is saved...'

// TODO: ensure REPL log is saved (flushed) to file before closing the REPL

Check warning on line 1069 in lib/node_modules/@stdlib/repl/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files

Unexpected 'todo' comment: 'TODO: ensure REPL log is saved (flushed)...'

nextTick( onTick );

Expand All @@ -1088,7 +1089,7 @@

// If this is a non-sandboxed REPL, remove global variables/properties which were introduced during context creation and by a user during a REPL session...
if ( self._sandbox === false ) {
// WARNING: in a non-sandboxed environment, if a global variable is externally introduced during a REPL session (i.e., introduced via a mechanism outside of the REPL environment), we will delete that global variable, which means the following logic may introduce unintended side-effects for this particular edge case (e.g., application code may expect the presence of the subsequently deleted global variable). While not ideal, (a) user applications should not be introducing globals to begin with and (b) the probability of a user running a REPL session, a user closing that REPL session, AND a global variable being introduced between starting a REPL and closing the REPL should be negligible.

Check warning on line 1092 in lib/node_modules/@stdlib/repl/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files

Unexpected 'warning' comment: 'WARNING: in a non-sandboxed environment,...'
tmp = self._context.vars(); // current workspace variables
for ( i = 0; i < tmp.length; i++ ) {
if ( isConfigurableProperty( self._context, tmp[ i ] ) ) {
Expand Down Expand Up @@ -1168,7 +1169,7 @@
throw new Error( format( 'invalid argument. First argument must be a recognized setting. Value: `%s`.', name ) );
}
if ( nargs === 1 ) {
return this._settings[ name ]; // TODO: we should consider returning a deep copy if settings are allowed to be objects, not just primitives, in order to avoid unintentional mutation

Check warning on line 1172 in lib/node_modules/@stdlib/repl/lib/main.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files

Unexpected 'todo' comment: 'TODO: we should consider returning a...'
}
value = arguments[ 1 ];
f = SETTINGS_VALIDATORS[ SETTINGS[ name ].type ];
Expand Down
Loading