-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
docs: clarify usage cli options -e,-p on windows #15568
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Overall LGTM, but let's see what other collaborators will say about wording details.
doc/api/cli.md
Outdated
@@ -53,6 +53,11 @@ changes: | |||
Evaluate the following argument as JavaScript. The modules which are | |||
predefined in the REPL can also be used in `script`. | |||
|
|||
*Note*: Please use double quote for the `script`, although it does not matter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lukaszewczak Thank you for posting this.
I would suggest the following change:
*Note*: On Windows, using `cmd.exe` a single quote will not work correctly
because it only recognizes double `"` for quoting. In Powershell or
Git bash, both `'` and `"` are usable.
- Start with
On Windows
so it's clear that the note target Windows behaviur - IMHO there is no need to compare to Linux (also the comparison is with Linux'es shells, the OS does care)
- Mention that it's only
cmd.exe
, as PS and bash on windows understand'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @refack for your comment. I made a change in note according to your suggestions.
*Note*: On Windows, using `cmd.exe` a single quote will not work correctly | ||
because it only recognizes double `"` for quoting. In Powershell or | ||
Git bash, both `'` and `"` are usable. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny nit: can you please remove this spurious new line?
Edit: ignore me it seems to be consistent with the rest of the file.
PR-URL: #15568 Fixes: #15522 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Landed in 7e382c1 |
PR-URL: #15568 Fixes: #15522 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #15568 Fixes: #15522 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#15568 Fixes: nodejs/node#15522 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #15568 Fixes: #15522 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #15568 Fixes: #15522 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fixes: #15522
I open previously Pull Request (#15543), but I decided to close it because I messed a little with a git rebase... I'm sorry for this, but it seams I need to practice more with git..
About the change. So I put note about using double quote for the
script
only under option-e
, because description under-p
refers to option-e
so if the user would like to know the whole meaning about option-p
will read description under-e
with this new note.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
doc