Skip to content

child-process: support any shells on Windows #21943

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

Closed
tkamenoko wants to merge 16 commits into from
Closed

child-process: support any shells on Windows #21943

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
b42be28
child-process: fix switches for alternative shells on Windows
tkamenoko Jul 21, 2018
e9863e6
child-process: fix lint errors
tkamenoko Jul 21, 2018
6f3bccc
test: add exec-any-shells test
tkamenoko Jul 21, 2018
6b9b090
test: fix exec-any-shell test for powershell
tkamenoko Jul 23, 2018
1dcea34
child-process: enable to accept full path that contains spaces
tkamenoko Jul 23, 2018
c7163f8
child-process: undo unrelated style changes
tkamenoko Jul 24, 2018
a314340
test: remove hard coding path
tkamenoko Jul 26, 2018
ce7999a
test: fix spawnsync-shell to check correct options on Windows
tkamenoko Jul 26, 2018
097c905
child-process: set windowsVerbatimArguments true for any shell
tkamenoko Aug 2, 2018
a4122b7
child-process: use case-insensitive comparison to check shell name
tkamenoko Aug 2, 2018
27ba48f
test: add testcases to ensure uppercase shell name and complex commands
tkamenoko Aug 2, 2018
f93c796
test: fix to accept multiple lines output of `where bash`
tkamenoko Aug 2, 2018
ade01d9
test: revert windowsVerbatim flag, fix `isCmd` check in spawnsync-shell
tkamenoko Aug 2, 2018
5a2c130
doc: update shell requirements in child_process
tkamenoko Aug 7, 2018
e2192a3
fixup: windowsVerbatimArguments
joaocgreis Aug 14, 2018
7a10600
fixup: don't exhaust exec buffer
joaocgreis Sep 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
child-process: use case-insensitive comparison to check shell name
  • Loading branch information
@tkamenoko @joaocgreis
tkamenoko authored and joaocgreis committed Sep 1, 2018
commit a4122b7f10cb0f72a0793d636a25218ab391d0f4
2 changes: 1 addition & 1 deletion lib/child_process.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ function normalizeSpawnArguments(file, args, options) {
else
file = process.env.comspec || 'cmd.exe';
// '/d /s /c' is used only for cmd.exe.
if (file.endsWith('cmd.exe') || file.endsWith('cmd')) {
if (/^(?:.*\\)?cmd(?:\.exe)?$/i.test(file)) {
Copy link
Copy Markdown
Contributor

@refack refack Aug 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A late comment, I think that \b might be a simpler test for the prefix:

if (/\bcmd(\.exe)?$/i.test(file))

also IMHO no need to ?: the groups since we just .test

Copy link
Copy Markdown
Contributor

@refack refack Aug 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P.S. I would call my comment a "nit" (that is a non blocking style change)

args = ['/d', '/s', '/c', `"${command}"`];
} else {
args = ['-c', command];
Expand Down