Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@Trott
Copy link
Member

@Trott Trott commented Jun 1, 2016

Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

debugger

Description of change

The command line flag --debug-brk was ignored when the -e flag was
also present. This change allows the flags to both be honored when they
are used in a single command line.

Fixes: #3589

This changes some stack traces due to its modifications to lib/internal/bootstrap_node.js. Not sure if that's considered undesirable or not, but I imagine it could be modified to avoid that. See changes to expected output of messages tests to see what the changes are.

Not sure if this fixes a valid use case or not, but there's an issue open for it and there seems to be a history. @bnoordhuis surely knows more.

R=@bnoordhuis? @indutny?

@Trott Trott added the debugger label Jun 1, 2016
@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Jun 1, 2016
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: fold this into the previous if statement like you do with the outermost if statement. Saves a level of indent.

@bnoordhuis
Copy link
Member

LGTM. We have the means to fix the setTimeout hack now so that would be a good next step.

@cjihrig
Copy link
Contributor

cjihrig commented Jun 1, 2016

LGTM

The command line flag `--debug-brk` was ignored when the `-e` flag was
also present. This change allows the flags to both be honored when they
are used in a single command line.

Fixes: nodejs#3589
@Trott
Copy link
Member Author

Trott commented Jun 4, 2016

Trott added a commit to Trott/io.js that referenced this pull request Jun 4, 2016
The command line flag `--debug-brk` was ignored when the `-e` flag was
also present. This change allows the flags to both be honored when they
are used in a single command line.

PR-URL: nodejs#7089
Fixes: nodejs#3589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott
Copy link
Member Author

Trott commented Jun 4, 2016

Landed in 1a21524

@Trott Trott closed this Jun 4, 2016
evanlucas pushed a commit that referenced this pull request Jun 15, 2016
The command line flag `--debug-brk` was ignored when the `-e` flag was
also present. This change allows the flags to both be honored when they
are used in a single command line.

PR-URL: #7089
Fixes: #3589
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@evanlucas evanlucas mentioned this pull request Jun 16, 2016
@MylesBorins
Copy link
Contributor

@Trott lts?

@Trott
Copy link
Member Author

Trott commented Jul 11, 2016

@thealphanerd If it lands cleanly, yes.

@MylesBorins
Copy link
Contributor

this one didn't land cleanly so sad 😿

@Trott Trott deleted the debugeval branch January 13, 2022 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

debugger: --debug-brk should pause until debug client connects

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.