Skip to content

Loading…

debugger: also exit when the repl emits 'exit' #5637

Closed
wants to merge 1 commit into from

5 participants

@fb55

Fixes #5631

@bnoordhuis
Node.js Foundation member
@Raynos

:+1: This will help with a wrapper around the node debugger that I'm writing.

@cjihrig cjihrig commented on the diff
lib/_debugger.js
@@ -1552,7 +1551,16 @@ Interface.prototype.repl = function() {
// Exit debug repl
self.exitRepl();
- });
+
+ self.repl.rli.removeListener('SIGINT', exitDebugRepl);
+ self.repl.removeListener('exit', exitDebugRepl);
+ }
+
+ // Exit debug repl on Ctrl + C
+ this.repl.rli.on('SIGINT', exitDebugRepl);
@cjihrig Node.js Foundation member
cjihrig added a note

Could you use once() instead of on() so that you don't need to worry about removing the listener?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cjihrig cjihrig commented on the diff
lib/_debugger.js
@@ -1552,7 +1551,16 @@ Interface.prototype.repl = function() {
// Exit debug repl
self.exitRepl();
- });
+
+ self.repl.rli.removeListener('SIGINT', exitDebugRepl);
+ self.repl.removeListener('exit', exitDebugRepl);
+ }
+
+ // Exit debug repl on Ctrl + C
+ this.repl.rli.on('SIGINT', exitDebugRepl);
+
+ // Exit debug repl on '.exit'
+ this.repl.on('exit', exitDebugRepl);
@cjihrig Node.js Foundation member
cjihrig added a note

Same here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cjihrig
Node.js Foundation member

Ah, nevermind, using on() is fine here. LGTM

@jasnell
Node.js Foundation member

@cjihrig ... this is targeted at master. Do we want to push this off to nodejs/node or get this landed in v0.12?

@cjihrig
Node.js Foundation member

@jasnell let's take it to nodejs/node.

@jasnell
Node.js Foundation member

@fb55... I know it's been a while, but are you willing to update this and open it against master in http://github.com/nodejs/node?

@jasnell jasnell self-assigned this
@fb55 fb55 referenced this pull request in nodejs/node
Closed

debugger: also exit when the repl emits 'exit' #2369

@fb55

@jasnell /done

@jasnell
Node.js Foundation member

Thanks!

@jasnell jasnell closed this
@jasnell jasnell added a commit to nodejs/node that referenced this pull request
@fb55 fb55 debugger: also exit when the repl emits 'exit'
Exit the debug repl when repl emits 'exit'

Refs: nodejs/node-v0.x-archive#5637
Fixes: nodejs/node-v0.x-archive#5631
PR-URL: #2369
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
a95eb5c
@Fishrock123 Fishrock123 added a commit to nodejs/node that referenced this pull request
@fb55 fb55 debugger: also exit when the repl emits 'exit'
Exit the debug repl when repl emits 'exit'

Refs: nodejs/node-v0.x-archive#5637
Fixes: nodejs/node-v0.x-archive#5631
PR-URL: #2369
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
66dccaf
@Fishrock123 Fishrock123 added a commit to Fishrock123/node that referenced this pull request
@fb55 fb55 debugger: also exit when the repl emits 'exit'
Exit the debug repl when repl emits 'exit'

Refs: nodejs/node-v0.x-archive#5637
Fixes: nodejs/node-v0.x-archive#5631
PR-URL: nodejs#2369
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
4923e10
@TheAlphaNerd TheAlphaNerd added a commit to nodejs/node that referenced this pull request
@fb55 fb55 debugger: also exit when the repl emits 'exit'
Exit the debug repl when repl emits 'exit'

Refs: nodejs/node-v0.x-archive#5637
Fixes: nodejs/node-v0.x-archive#5631
PR-URL: #2369
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
d10b710
@TheAlphaNerd TheAlphaNerd added a commit to nodejs/node that referenced this pull request
@fb55 fb55 debugger: also exit when the repl emits 'exit'
Exit the debug repl when repl emits 'exit'

Refs: nodejs/node-v0.x-archive#5637
Fixes: nodejs/node-v0.x-archive#5631
PR-URL: #2369
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
31b4091
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 7, 2013
  1. @fb55
Showing with 12 additions and 4 deletions.
  1. +12 −4 lib/_debugger.js
View
16 lib/_debugger.js
@@ -1540,9 +1540,8 @@ Interface.prototype.repl = function() {
// Don't display any default messages
var listeners = this.repl.rli.listeners('SIGINT').slice(0);
this.repl.rli.removeAllListeners('SIGINT');
-
- // Exit debug repl on Ctrl + C
- this.repl.rli.once('SIGINT', function() {
+
+ function exitDebugRepl(){
// Restore all listeners
process.nextTick(function() {
listeners.forEach(function(listener) {
@@ -1552,7 +1551,16 @@ Interface.prototype.repl = function() {
// Exit debug repl
self.exitRepl();
- });
+
+ self.repl.rli.removeListener('SIGINT', exitDebugRepl);
+ self.repl.removeListener('exit', exitDebugRepl);
+ }
+
+ // Exit debug repl on Ctrl + C
+ this.repl.rli.on('SIGINT', exitDebugRepl);
@cjihrig Node.js Foundation member
cjihrig added a note

Could you use once() instead of on() so that you don't need to worry about removing the listener?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ // Exit debug repl on '.exit'
+ this.repl.on('exit', exitDebugRepl);
@cjihrig Node.js Foundation member
cjihrig added a note

Same here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
// Set new
this.repl.eval = this.debugEval.bind(this);
Something went wrong with that request. Please try again.