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.

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

doc: add missing windowsVerbatimArguments option in child_process.spawn #7884

Closed
wants to merge 2 commits into from

Conversation

stevemao
Copy link
Contributor

@stevemao stevemao commented Jul 26, 2016

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

Windows

Description of change

Just documentation change for windowsVerbatimArguments option in child_process.spawn

/cc @nodejs/documentation @SetTrend

Fixes #5060

@nodejs-github-bot nodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. labels Jul 26, 2016
@mscdex
Copy link
Contributor

mscdex commented Jul 26, 2016

This is actually only adding it to the documentation for .fork(). Technically if it's going to be added, it should be added to all of the methods.

@cjihrig
Copy link
Contributor

cjihrig commented Aug 1, 2016

All of the methods except exec() and execSync() should specify that the default is false. exec() and execSync() unconditionally set this to true on Windows, and false otherwise.

@garthk
Copy link

garthk commented Aug 9, 2016

@cjihrig what's the mechanism for "unconditionally set this to true"? In 99f45b2 I see exec call normalizeExecArgs, which passes options objects through without touching windowsVerbatimArguments, then execFile, which coerces its options. windowsVerbatimArguments to a boolean, i.e. false unless given.

@cjihrig
Copy link
Contributor

cjihrig commented Aug 9, 2016

normalizeExecArgs(), which is only called from exec() and execSync(), sets the shell option. normalizeSpawnArguments() detects the shell option, and on Windows sets windowsVerbatimArguments to true.

@garthk
Copy link

garthk commented Aug 10, 2016

Aah, got it. Thanks.

@MylesBorins
Copy link
Contributor

@stevemao do you still want to run with this? if not we should make it a good first contribution

@jasnell jasnell added the stalled Issues and PRs that are stalled. label Mar 1, 2017
@fhinkel
Copy link
Member

fhinkel commented Mar 26, 2017

ping @stevemao

@jasnell
Copy link
Member

jasnell commented Aug 24, 2017

Closing due to lack of forward progress. We can reopen if necessary

@jasnell jasnell closed this Aug 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. stalled Issues and PRs that are stalled.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quotes are not handled correctly when child_process.spawn() parses args
8 participants