Skip to content

Windows support#10

Closed
seangenabe wants to merge 1 commit intoxxorax:masterfrom
seangenabe:win32
Closed

Windows support#10
seangenabe wants to merge 1 commit intoxxorax:masterfrom
seangenabe:win32

Conversation

@seangenabe
Copy link

Enables Windows support (#9)

@andersk
Copy link

andersk commented Jul 13, 2017

The corresponding code in cross-spawn looks a lot simpler.

@binki
Copy link

binki commented Jul 14, 2017

Because escaping arguments is not the only issue with launching processes portably on different platforms, I recommend to just use cross-spawn and its spawn() (which accepts an array of arguments) instead of this package.

@andersk
Copy link

andersk commented Jul 14, 2017

@binki Absolutely—but in more complicated use cases like passing a command to a command, additional layers of manual shell quoting may still be required.

Some of these complicated use cases, like running a command over SSH on a Unix server from a Windows client, may even require shell quoting for a platform other than the current one. So I would suggest providing three functions, one for Windows, one for Unix, and one that switches on the current platform.

@xxorax
Copy link
Owner

xxorax commented Jan 27, 2018

Sorry for late reply. This lib is not intended to support Windows shell like this.

This pull request escape shell args based on the current system, which is maybe not what we want (how about escaping to execute a command on an other system ? ex: by ssh).

So it should set the shell type with some option. That means redefining the API.

I'm open to it, maybe sub-functions to escape string by shell (like shellescape.sh(), shellescape.bash(). shellescape.powershell(), ..etc).

Also note that the code in cross-spawn mentionned in the comment below does not escape anything inside the double-quotes except quotes (and so it will not pass the current tests of node-shell-escape).

@xxorax xxorax closed this Jan 27, 2018
This was referenced Jan 27, 2018
@andersk
Copy link

andersk commented Jan 27, 2018

Also note that the code in cross-spawn mentionned in the comment below does not escape anything inside the double-quotes except quotes (and so it will not pass the current tests of node-shell-escape).

To be clear, it also escapes any backslashes inside double-quotes that belong to a sequence of backslashes preceding an escaped double-quote or the end of the string. (This is wrong for Unix, but is the only right thing for Windows, where doubled backslashes are only undoubled within such sequences.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants