36

I'm getting a

sed: -e expression #1, char 22: unterminated `s' command 

is there a problem on my script? Also the "oldstring" has special characters

#!bin/bash
oldstring='<script>"[oldscript]"</script>'
newstring='<script>"[newscript]"</script>'
grep -rl $oldstring /path/to/folder | xargs sed -i s/$oldstring/$newstring/g
Zombo
  • 1
  • 62
  • 391
  • 407
jmazaredo
  • 493
  • 1
  • 4
  • 9
  • 1
    The square brackets may need to be escaped. Also double quote the sed pattern. – phs Apr 10 '13 at 08:08
  • 3
    You'll also have an issue with the `/` in your patterns. They should be escaped. Or you could use another delimiter for your `s` command (e.g. `@`). – Didier Trosset Apr 10 '13 at 08:13

5 Answers5

52

As @Didier said, you can change your delimiter to something other than /:

grep -rl $oldstring /path/to/folder | xargs sed -i s@$oldstring@$newstring@g
wudeng
  • 795
  • 5
  • 8
  • 7
    The above would fail in SO many interesting ways given so many possible contents of oldstring or newstring. – Ed Morton Apr 11 '13 at 13:06
  • 1
    @EdMorton, would you please put an example of how does it fail? I know you posted the traditional form of doing below, but just saying it can fail leave the question of how. Then I could understand why I should your solution. These solution just seems much more easy. Thanks – toto_tico Mar 13 '14 at 07:06
  • 3
    Try the above with any shell globbing character or an @ in either string variable. The globbing chars can/will match file names in your directory, if not today then some day in future when you last expect it, and the @ will terminate your sed command. It'll also fail if oldstring contains spaces in various locations, not to mention if either string contains newline characters. The OP specifically said `the "oldstring" has special characters` so that's a big heads up that any of what I just described and more is likely to happen. – Ed Morton Mar 13 '14 at 13:22
  • 3
    Also take care that in some environments, notably Darwin/MacOSX you need to give an explicit backup extension like 'sed -i .bak'. – thoni56 Feb 17 '15 at 07:25
  • @thoni56 "notably Darwin/MacOSX" Also known as BSD... – Dev Jan 09 '21 at 20:36
13
grep -rl $oldstring . | xargs sed -i "s/$oldstring/$newstring/g"
Weafs.py
  • 22,731
  • 9
  • 56
  • 78
Sreedhar GS
  • 2,694
  • 1
  • 24
  • 26
  • So the complication with this, is files with spaces in their names won't be picked up. If you had "/files/File Two.txt", sed would try "/files/File" and "Two.txt". – Captainlonate Oct 02 '17 at 02:54
6

The GNU guys REALLY messed up when they introduced recursive file searching to grep. grep is for finding REs in files and printing the matching line (g/re/p remember?) NOT for finding files. There's a perfectly good tool with a very obvious name for FINDing files. Whatever happened to the UNIX mantra of do one thing and do it well?

Anyway, here's how you'd do what you want using the traditional UNIX approach (untested):

find /path/to/folder -type f -print |
while IFS= read -r file
do
   awk -v old="$oldstring" -v new="$newstring" '
      BEGIN{ rlength = length(old) }
      rstart = index($0,old) { $0 = substr($0,rstart-1) new substr($0,rstart+rlength) }
      { print }
   ' "$file" > tmp &&
   mv tmp "$file"
done

Not that by using awk/index() instead of sed and grep you avoid the need to escape all of the RE metacharacters that might appear in either your old or your new string plus figure out a character to use as your sed delimiter that can't appear in your old or new strings, and that you don't need to run grep since the replacement will only occur for files that do contain the string you want. Having said all of that, if you don't want the file timestamp to change if you don't modify the file, then just do a diff on tmp and the original file before doing the mv or throw in an fgrep -q before the awk.

Caveat: The above won't work for file names that contain newlines. If you have those then let us know and we can show you how to handle them.

Ed Morton
  • 188,023
  • 17
  • 78
  • 185
  • 2
    +1 When someone talks about using `sed` with `grep`, there is an excellent chance they should be using just `sed` or switch to `awk`. By the way, if you used `-print0` in your find, and (in BASH) using `-d\0` should take care of file names with new lines in them. – David W. Apr 11 '13 at 13:02
  • @DavidW. Right. And when you start talking about escaping RE metacharacters so that sed or grep doesn't treat them as such, then you need to switch to awk/index() or fgrep so you can look for strings instead of REs. – Ed Morton Apr 11 '13 at 13:05
  • @josten the `-r` arg for `grep` is an alternative to using `find` not to using `awk`. You can use `awk` with `find+xargs` instead of `grep+sed` and the result will usually be more concise and efficient. Using `awk` will certainly not cause you to create huge convoluted one-liners. – Ed Morton Feb 15 '15 at 19:41
4

sed expression needs to be quoted

sed -i "s/$oldstring/$newstring/g"
Zombo
  • 1
  • 62
  • 391
  • 407
-1
grep -rl SOSTITUTETHIS . | xargs sed -Ei 's/(.*)SOSTITUTETHIS(.*)/\1WITHTHIS\2/g'
Mok
  • 35
  • 4