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

child_process,Windows: fix docs for spawn({shell}) #14156

Closed
refack opened this issue Jul 10, 2017 · 4 comments
Closed

child_process,Windows: fix docs for spawn({shell}) #14156

refack opened this issue Jul 10, 2017 · 4 comments
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.

Comments

@refack
Copy link
Contributor

refack commented Jul 10, 2017 •
edited

  • Version: *
  • Platform: Windows
  • Subsystem: child_process

https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options erronously states that cmd.exe is the default shell for windows, while infact it's process.env.ComSpec.
The same goes for:
@refack refack added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. labels Jul 10, 2017
@refack refack mentioned this issue Jul 10, 2017
Closed
2 tasks
@refack
Copy link
Contributor Author

refack commented Jul 10, 2017

/cc @nodejs/platform-windows @nodejs/documentation

@tniessen
Copy link
Member

This is one of many problems arising from setting weird environments for child processes.

Technically, the documentation is not entirely incorrect. Microsoft specifies that COMSPEC must always contain the path to cmd.exe, at least for the root environment, so as long as that is true, the documentation is correct. It is still worth changing this in the documentation as - sadly - it is not always true for child processes...

@cjihrig
Copy link
Contributor

cjihrig commented Jul 10, 2017

I think we should say that the value defaults to COMSPEC, and if that is not set, cmd.exe is used.

@tniessen
Copy link
Member

Ref #14157

henryzxu added a commit to henryzxu/node that referenced this issue Jul 12, 2017
@henryzxu
doc: fixes default shell in child_process.md
1a07c2a 
Clarifies the default shell in Windows is process.env.ComSpec
and that cmd.exe is only used as a fallback. Functions whose
descriptions are affected include:
child_process.spawn, child_process.exec,
child_process.spawnsSync, and child_process.execSync.

Fixes: nodejs#14156
@henryzxu henryzxu mentioned this issue Jul 12, 2017
Closed
2 tasks
henryzxu added a commit to henryzxu/node that referenced this issue Jul 12, 2017
@henryzxu
doc: fixes default shell in child_process.md
e9089d3 
Clarifies that the default shell in Windows is process.env.ComSpec
and that cmd.exe is only used as a fallback. Functions whose
descriptions are affected include:
child_process.spawn, child_process.exec,
child_process.spawnSync, and child_process.execSync.

Fixes: nodejs#14156
henryzxu added a commit to henryzxu/node that referenced this issue Jul 17, 2017
@henryzxu
doc: fixes default shell in child_process.md
fd92d17 
Reorganizes `child_process` shell spawning information.

Fixes: nodejs#14156
henryzxu added a commit to henryzxu/node that referenced this issue Jul 17, 2017
@henryzxu
doc: fixes default shell in child_process.md
33adbeb 
Small formatting fixes for shell spawning information.

Fixes: nodejs#14156
henryzxu added a commit to henryzxu/node that referenced this issue Jul 18, 2017
@henryzxu
doc: fixes default shell in child_process.md
e4171c3 
Notation changes for `process.env.ComSpec`

Fixes: nodejs#14156
henryzxu added a commit to henryzxu/node that referenced this issue Jul 18, 2017
@henryzxu
doc: fixes default shell in child_process.md
46a8829 
Wrapped `%COMSPEC%` with percent signs.

Fixes: nodejs#14156
addaleax pushed a commit that referenced this issue Jul 22, 2017
@henryzxu @addaleax
doc: fixes default shell in child_process.md
5b3dc85 
Clarifies the default shell in Windows is process.env.ComSpec
and that cmd.exe is only used as a fallback. Functions whose
descriptions are affected include:
child_process.spawn, child_process.exec,
child_process.spawnsSync, and child_process.execSync.

PR-URL: #14203
Fixes: #14156
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Fishrock123 pushed a commit that referenced this issue Jul 24, 2017
@henryzxu @Fishrock123
doc: fixes default shell in child_process.md
4be373b 
Clarifies the default shell in Windows is process.env.ComSpec
and that cmd.exe is only used as a fallback. Functions whose
descriptions are affected include:
child_process.spawn, child_process.exec,
child_process.spawnsSync, and child_process.execSync.

PR-URL: #14203
Fixes: #14156
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

doc: fixes default shell in child_process.md henryzxu/node
3 participants
@refack @cjihrig @tniessen