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.

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

src: indent long help text properly #37911

Closed
wants to merge 1 commit into from
Closed

src: indent long help text properly #37911

wants to merge 1 commit into from

Conversation

glasser
Copy link
Contributor

@glasser glasser 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-bot nodejs-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
@glasser glasser mentioned this pull request Mar 25, 2021
Merged
@glasser
Copy link
Contributor Author

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.

@addaleax addaleax 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

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]>
@aduh95 aduh95 force-pushed the wrapped-help-indentation branch from a6ed940 to 19ed27d Compare March 28, 2021 17:30
@aduh95
Copy link
Contributor

aduh95 commented Mar 28, 2021

Landed in 19ed27d

@aduh95 aduh95 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]>
@ruyadorno ruyadorno 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]>
@targos targos 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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RaisinTen RaisinTen RaisinTen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs 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