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

lib: Improve readability of help text in node --help #23016

Merged
merged 1 commit into from
Sep 28, 2018
Merged

lib: Improve readability of help text in node --help #23016

merged 1 commit into from
Sep 28, 2018

Conversation

gireeshpunathil
Copy link
Member

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@gireeshpunathil build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/971/pipeline

@addaleax
Copy link
Member

I think the current text was intentional; reading from stdin is the default when no other file name is provided, and if stdin is a TTY we enter interactive mode?

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Sep 22, 2018 •
edited
Loading

It seems the "reading from stdin" is wider than "interactive mode" (it may be a command chain), so this is a note about a partial case, not the definition of "default", and the semicolon seems valid.

@gireeshpunathil
Copy link
Member Author

ah! I see it now, thanks for the clarification.

@addaleax
Copy link
Member

@gireeshpunathil That doesn’t mean we can’t improve here … I agree that this is not immediately obvious. Maybe we could change it to something like default if no file name is provided; …?

@gireeshpunathil
Copy link
Member Author

I reworded the help message for better readability, PTAL.

@gireeshpunathil gireeshpunathil changed the title lib: fix punctuation in help text lib: Improve readability of help text in node --help Sep 22, 2018
@gireeshpunathil
Copy link
Member Author

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

@danbev
Copy link
Contributor

danbev commented Sep 25, 2018 •
edited
Loading

Re-run of failing node-test-commit-linux-containered and node-test-commit-linux/

@joyeecheung joyeecheung mentioned this pull request Sep 25, 2018
Closed
@gireeshpunathil
Copy link
Member Author

rebased and started a new CI: https://ci.nodejs.org/job/node-test-pull-request/17491/

@gireeshpunathil
Copy link
Member Author

only one known failure under node-test-commit-linux - not ok 46 async-hooks/test-statwatcher # TODO : Fix flaky test, so good to land.

@gireeshpunathil gireeshpunathil added the cli Issues and PRs related to the Node.js command line interface. label Sep 28, 2018
@gireeshpunathil
lib: reword help text for clarity
0f78b9a 
modified the `-` CLI option help text for
improved readability.

PR-URL: nodejs#23016

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@gireeshpunathil gireeshpunathil merged commit 0f78b9a into nodejs:master Sep 28, 2018
targos pushed a commit that referenced this pull request Sep 28, 2018
@gireeshpunathil @targos
lib: reword help text for clarity
60924b7 
modified the `-` CLI option help text for
improved readability.

PR-URL: #23016

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Oct 3, 2018
@gireeshpunathil @targos
lib: reword help text for clarity
c40e2dd 
modified the `-` CLI option help text for
improved readability.

PR-URL: #23016

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@targos targos mentioned this pull request Oct 7, 2018
Merged
This was referenced Oct 10, 2018
Closed
Closed
Closed
Closed
This was referenced Oct 11, 2018
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

@lpinca lpinca lpinca approved these changes

@addaleax addaleax addaleax approved these changes

@thefourtheye thefourtheye thefourtheye approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
cli Issues and PRs related to the Node.js command line interface.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@gireeshpunathil @nodejs-github-bot @addaleax @vsemozhetbyt @danbev @jasnell @thefourtheye @lpinca @BridgeAR