shelljs / shx

Portable Shell Commands for Node
MIT License
1.72k stars 44 forks source link

sed can't replace files in a different directory #165

Closed crhistianramirez closed 5 years ago

crhistianramirez commented 5 years ago

shx version: 0.3.2 npm version: 6.9.0 node version: 10.16.0

I'm trying to replace some text for a file in a child folder /docs.

I run this command:

shx sed -i 's/globals.html/index.html/g' docs/index.html

And get the error: sed: no files given

I've confirmed this works with the vanilla unix sed command. Steps to reproduce:

  1. git clone https://github.com/crhistianramirez/shx-bug-repro-sed.git
  2. cd shx-bug-repro-sed
  3. npm install
  4. npm run bug
nfischer commented 5 years ago

Does it work correctly like this?

shx sed -i 's/globals.html/index.html/g' index.html # file in same folder
crhistianramirez commented 5 years ago

No, it does not. I get the same error

sed: no files given
nfischer commented 5 years ago

Which operating system? I can't repro on Linux.

crhistianramirez commented 5 years ago

Ah, I'm using Windows 10

nfischer commented 5 years ago

Can you check if the corresponding shelljs method call works? I don't have a windows box to repro.

crhistianramirez commented 5 years ago

Made one more commit - https://github.com/crhistianramirez/shx-bug-repro-sed.git

and was not able to reproduce with shelljs. The following code successfully replaces the text as I expect it to:

const shell = require('shelljs');
shell.sed('-i', /globals.html/g, 'index.html', 'docs/index.html');

So it looks like it has to do with how the parameters are getting passed to the cli. I'll see if I can dig into that code a bit more over the weekend.

crhistianramirez commented 5 years ago

Tracked this down to a bug with how minimist is parsing the arguments. Seems to be escaping the single quotes when the environment is windows and the command is run from a node script. I've submitted a bug report but the last commit to that repository was 4 years ago.

This command: shx sed -i 's/globals.html/index.html/g' docs/index.html while run as a node script yields the following parsed arguments from minimist.

WINDOWS

{ _: [ 'sed', 'index.html' ], i: '\'s/globals.html/index.html/g\'' }

MAC

{ _: [ 'sed', 'index.html' ], i: 's/globals.html/index.html/g' }
nfischer commented 5 years ago

Are you sure it's minimist at fault? Isn't that because of what process.argv looks like, because Windows sends single quotes to the child process argv?

crhistianramirez commented 5 years ago

🤦‍♂ Yep, that was it.

Hmm kind of a bummer that windows handles it that way. So it looks like maybe just escaping double quotes is the way to go. This command works from an npm script:

shx sed -i \"s/globals.html/index.html/g\" docs/index.html

Might be good to add this to the docs since shx is meant primarily to be run from an npm script. Mind if I add it?

nfischer commented 5 years ago

Ah I see, this is in package.json like so:

{
  "scripts": {
    "foo": "shx sed -i \"s/globals.html/index.html/g\" docs/index.html"
  }
}

Now I see the appeal of single quotes, although I think we've found those don't work.

For your specific case, you can omit quotes entirely:

    "foo": "shx sed -i s/globals.html/index.html/g docs/index.html"

But in general, yeah this is something we might document.

crhistianramirez commented 5 years ago

Ah didn't know that, more my ignorance I'm sure. I've just started playing around with the command line.

I'll close this issue then. Thank you very much for your help @nfischer it was very much appreciated :)

nfischer commented 4 years ago

Following up here to summarize the discussion, for future reference. Be careful with single quotes. Windows and unix treat single quotes differently and there's no way for shx to workaround this. Prefer double quotes instead when possible, even if that means escaping double quotes inside package.json.