Skip to content

src: indent long help text properly#37911

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

Jump to bottom
Closed
glasser wants to merge 1 commit into from
Closed

src: indent long help text properly #37911

glasser wants to merge 1 commit into from
+3 −3

Conversation

@glasser
Copy link
Contributor

@glasserglasser commented Mar 25, 2021•
edited
Loading

The previous code passed an ignored argument to StringPrototypeTrimLeft, and
tried to trim a string that didn't start with whitespace. The trim makes more
sense after the indentation has been added. Now wrapped lines actually show up
with the rest of the help text.

This was a regression introduced in #36140.

Before:

 --frozen-intrinsics experimental frozen intrinsics support --heap-prof Start the V8 heap profiler on start up, and write the heap profile to disk before exit. If --heap-prof-dir is not specified, write the profile to the current working directory. --heap-prof-dir=... Directory where the V8 heap profiles generated by --heap-prof will be placed.

After:

 --frozen-intrinsics experimental frozen intrinsics support --heap-prof Start the V8 heap profiler on start up, and write the heap profile to disk before exit. If --heap-prof-dir is not specified, write the profile to the current working directory. --heap-prof-dir=... Directory where the V8 heap profiles generated by --heap-prof will be placed.

Doing this made an uncharacteristic trailing newline in the --icu-data-dir
help text more obvious, so I removed that.

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Mar 25, 2021
@glasserglasser mentioned this pull request Mar 25, 2021
Merged
addaleax
addaleax reviewed Mar 25, 2021
View reviewed changes
@glasser
Copy link
ContributorAuthor

glasser commented Mar 25, 2021

Taking a closer look at the history, this was a regression in #36140, where the order of the two functions was accidentally reversed.

@addaleaxaddaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Mar 27, 2021
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 27, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/37022/

@glasser@aduh95
src: indent long help text properly
19ed27d
The previous code passed an ignored argument to StringPrototypeTrimLeft, and tried to trim a string that didn't start with whitespace. The trim makes more sense after the indentation has been added. Now wrapped lines actually show up with the rest of the help text. Doing this made an uncharacteristic trailing newline in the `--icu-data-dir` help text more obvious, so I removed that. PR-URL: nodejs#37911 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: James M Snell <[email protected]>
@aduh95aduh95force-pushed the wrapped-help-indentation branch from a6ed940 to 19ed27dCompareMarch 28, 2021 17:30
@aduh95
Copy link
Contributor

aduh95 commented Mar 28, 2021

Landed in 19ed27d

@aduh95aduh95 closed this Mar 28, 2021
ruyadorno pushed a commit that referenced this pull request Mar 29, 2021
@glasser@ruyadorno
src: indent long help text properly
ff3d20c
The previous code passed an ignored argument to StringPrototypeTrimLeft, and tried to trim a string that didn't start with whitespace. The trim makes more sense after the indentation has been added. Now wrapped lines actually show up with the rest of the help text. Doing this made an uncharacteristic trailing newline in the `--icu-data-dir` help text more obvious, so I removed that. PR-URL: #37911 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: James M Snell <[email protected]>
ruyadorno pushed a commit that referenced this pull request Mar 30, 2021
@glasser@ruyadorno
src: indent long help text properly
b6ad8e4
The previous code passed an ignored argument to StringPrototypeTrimLeft, and tried to trim a string that didn't start with whitespace. The trim makes more sense after the indentation has been added. Now wrapped lines actually show up with the rest of the help text. Doing this made an uncharacteristic trailing newline in the `--icu-data-dir` help text more obvious, so I removed that. PR-URL: #37911 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: James M Snell <[email protected]>
@ruyadornoruyadorno mentioned this pull request Mar 30, 2021
Merged
targos pushed a commit that referenced this pull request May 30, 2021
@glasser@targos
src: indent long help text properly
d2926d8
The previous code passed an ignored argument to StringPrototypeTrimLeft, and tried to trim a string that didn't start with whitespace. The trim makes more sense after the indentation has been added. Now wrapped lines actually show up with the rest of the help text. Doing this made an uncharacteristic trailing newline in the `--icu-data-dir` help text more obvious, so I removed that. PR-URL: #37911 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request Jun 5, 2021
@glasser@targos
src: indent long help text properly
b407b99
The previous code passed an ignored argument to StringPrototypeTrimLeft, and tried to trim a string that didn't start with whitespace. The trim makes more sense after the indentation has been added. Now wrapped lines actually show up with the rest of the help text. Doing this made an uncharacteristic trailing newline in the `--icu-data-dir` help text more obvious, so I removed that. PR-URL: #37911 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: James M Snell <[email protected]>
@targostargos mentioned this pull request Jun 6, 2021
Merged
targos pushed a commit that referenced this pull request Jun 11, 2021
@glasser@targos
src: indent long help text properly
006c7b7
The previous code passed an ignored argument to StringPrototypeTrimLeft, and tried to trim a string that didn't start with whitespace. The trim makes more sense after the indentation has been added. Now wrapped lines actually show up with the rest of the help text. Doing this made an uncharacteristic trailing newline in the `--icu-data-dir` help text more obvious, so I removed that. PR-URL: #37911 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes

@addaleaxaddaleaxaddaleax approved these changes

@lpincalpincalpinca approved these changes

@cjihrigcjihrigcjihrig approved these changes

@RaisinTenRaisinTenRaisinTen approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@glasser@nodejs-github-bot@aduh95@jasnell@addaleax@lpinca@cjihrig@RaisinTen