Skip to content
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.

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

[v14.x backport] debugger patches #39446

Closed
wants to merge 24 commits into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jul 19, 2021

This should help unblock other debugger related backports.

@aduh95 aduh95 requested a review from Trott July 19, 2021 13:48
@github-actions github-actions bot added errors Issues and PRs related to JavaScript errors originated in Node.js core. inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run. v14.x labels Jul 19, 2021
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM with line-wrap lint fixes and a green CI

@richardlau

This comment has been minimized.

@aduh95 aduh95 force-pushed the backport-debugger-patches branch from 224c3d8 to f5ec086 Compare July 20, 2021 12:14
aduh95 and others added 10 commits July 20, 2021 15:18
PR-URL: nodejs#36532
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#38400
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Refs: nodejs#38273 (comment)

PR-URL: nodejs#38811
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
I'm not sure that this is any clearer than the existing code, but I
don't think it's significantly less clear, and it avoids comment
disabling a lint rule.

PR-URL: nodejs#38847
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Current code masks setInterval and setTimeout with promisified versions.
This can be confusing to read and causes lint errors. Replace masking
with use of pSetInterval and pSetTimeout instead.

Move disabling of lint rule from entire file to the one remaining line
(after the above changes) that still needs it.

PR-URL: nodejs#38946
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#38417
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#38952
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Node.js uses (or will use) _Ctrl+C_ with no spaces. To unify the
messages from inspect with other messages from the REPL and elsewhere,
make that change in node-inspect too.

PR-URL: nodejs#38400
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 22, 2021
PR-URL: #39024
Backport-PR-URL: #39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 22, 2021
PR-URL: #39024
Backport-PR-URL: #39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 22, 2021
PR-URL: #39024
Backport-PR-URL: #39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 22, 2021
PR-URL: #39024
Backport-PR-URL: #39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 22, 2021
PR-URL: #39024
Backport-PR-URL: #39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 22, 2021
When I moved these files from node-inspect to Node.js core, I put them
in lib/internal/inspector. That was a mistake. They should be in
lib/internal/debugger.

PR-URL: #39080
Backport-PR-URL: #39446
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@richardlau richardlau mentioned this pull request Jul 22, 2021
@aduh95 aduh95 deleted the backport-debugger-patches branch July 22, 2021 17:06
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#36532
Backport-PR-URL: nodejs#39446
Reviewed-By: Rich Trott <rtrott@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
This reverts commit d0c7ab0.

PR-URL: nodejs#39446
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#38400
Backport-PR-URL: nodejs#39446
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#38406
Backport-PR-URL: nodejs#39446
Reviewed-By: Rich Trott <rtrott@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
Refs: nodejs#38273 (comment)

PR-URL: nodejs#38811
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
I'm not sure that this is any clearer than the existing code, but I
don't think it's significantly less clear, and it avoids comment
disabling a lint rule.

PR-URL: nodejs#38847
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
Current code masks setInterval and setTimeout with promisified versions.
This can be confusing to read and causes lint errors. Replace masking
with use of pSetInterval and pSetTimeout instead.

Move disabling of lint rule from entire file to the one remaining line
(after the above changes) that still needs it.

PR-URL: nodejs#38946
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#38417
Backport-PR-URL: nodejs#39446
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#38952
Backport-PR-URL: nodejs#39446
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
Node.js uses (or will use) _Ctrl+C_ with no spaces. To unify the
messages from inspect with other messages from the REPL and elsewhere,
make that change in node-inspect too.

PR-URL: nodejs#38400
Backport-PR-URL: nodejs#39446
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
Wrap lines more than 80 chararcters long in inspect_repl.js so we can
disable specific rules.

PR-URL: nodejs#38529
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
Avoid non-ASCII char in lib code as a single non-ASCII char forces all
the chars to be stored inefficiently, bloating the binary size. This
also brings the file closer to compatibility with our lint rules.

PR-URL: nodejs#38529
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#38529
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
This avoids loading the entirety of `node:util` and `node:url` and their
dependencies while only a subset is actually used by this module.

PR-URL: nodejs#38550
Backport-PR-URL: nodejs#39446
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#38551
Backport-PR-URL: nodejs#39446
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#38850
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#39024
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#39024
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#39024
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#39024
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#39024
Backport-PR-URL: nodejs#39446
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
When I moved these files from node-inspect to Node.js core, I put them
in lib/internal/inspector. That was a mistake. They should be in
lib/internal/debugger.

PR-URL: nodejs#39080
Backport-PR-URL: nodejs#39446
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core. inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants