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

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

Closed
crhistianramirez opened this issue Aug 5, 2019 · 12 comments · Fixed by #170
Closed

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

crhistianramirez opened this issue Aug 5, 2019 · 12 comments · Fixed by #170

Comments

@crhistianramirez
Copy link

crhistianramirez commented Aug 5, 2019

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
Copy link
Member

Does it work correctly like this?

shx sed -i 's/globals.html/index.html/g' index.html # file in same folder

@crhistianramirez
Copy link
Author

No, it does not. I get the same error

sed: no files given

@nfischer
Copy link
Member

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

@crhistianramirez
Copy link
Author

Ah, I'm using Windows 10

@nfischer
Copy link
Member

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

@crhistianramirez
Copy link
Author

crhistianramirez commented Aug 24, 2019

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
Copy link
Author

crhistianramirez commented Aug 25, 2019

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
Copy link
Member

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
Copy link
Author

🤦‍♂ 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 nfischer added the docs label Aug 27, 2019
@nfischer
Copy link
Member

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
Copy link
Author

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
Copy link
Member

nfischer commented Dec 9, 2019

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.

nfischer added a commit that referenced this issue Dec 9, 2019
No change to logic, only docs.

This documents the special sed syntax (ex. `shx sed s/foo/bar/g
filename.txt`). With that, this mentions that `/` is a special
character, so the user must escape that if it's present in either the
regex or replacement string.

This also documents how Windows handles single quotes differently, so
it's desirable to avoid single quotes in package.json scripts. This part
isn't specific to sed, but since it's popular to quote sed expressions,
I'm adding this to the docs at the same time.

Fixes #165
Fixes #169
nfischer added a commit that referenced this issue Dec 9, 2019
No change to logic, only docs.

This documents the special sed syntax (ex. `shx sed s/foo/bar/g
filename.txt`). With that, this mentions that `/` is a special
character, so the user must escape that if it's present in either the
regex or replacement string.

This also documents how Windows handles single quotes differently, so
it's desirable to avoid single quotes in package.json scripts. This part
isn't specific to sed, but since it's popular to quote sed expressions,
I'm adding this to the docs at the same time.

Fixes #165
Fixes #169
n0samu added a commit to n0samu/locale-switcher that referenced this issue Jun 18, 2023
cupcakearmy pushed a commit to locale-switcher/locale-switcher that referenced this issue Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants