As mentioned before, you could make some things easier if you used bash
instead of sh
. Is it possible for you to do so, or does it have to be POSIX portable?
In any case I want to go over a few lines and cover some important coding points:
As mentioned, /bin/sh
forces the script to be interpreted according to POSIX standards. To (reliably) use another shell's features you need to explicitly specify it, i.e. #!/bin/bash
read -p "..." DIRROOT
This is a minor point, but since environment variables are generally all upper-case, it's good practice to keep your own user variables in lower-case or mixed-case to help differentiate them.
DATAROOT=$(grep -i '^$CFG->dataroot' $DIRROOT/config.php | sed "s|^.*'\(.*\)';.*|\1|")
a. As mentioned, single quotes don't allow variable expansion. But in addition, unquoted
variables ($DIRROOT) are subject to word splitting and glob expansion. This is a very important
scripting point, so learn it well!
b. There's usually no need to use chains of grep
, etc. sed
can do its own line matching, and awk
can do the job of both of the other two.
DATAROOT=$( sed -rn "/^$CFG->dataroot/I s|^.*'(.*)';.*|\1|p" "$DIRROOT/config.php" )
( I believe "I
" may be a gnu extension, however, and perhaps also the -r
option that lets you avoid having to backslash advanced regex syntax. )
I'm also a little concerned about your sed regex. Since "*
" is greedy, it may match more than you expect it to. But I'd have to see some examples of the text to say for sure.
c. You really should be running some tests on your user input to make sure that you were given proper file/directory names before you use them like this.
OLDTHEMENAME=$( echo "$d" | cut -d'/' -f1 )
You should be able to use parameter substitution
instead of cut
here. The basic ones should be available to all shells, at least.
if [! -d $NEWTHEMEDIR]; then
mkdir -p $NEWTHEMEDIR;
a. As mentioned, watch the spaces when using the test construct.
b. Again, be sure to properly quote your variables.
c. If you're using bash
, then it's recommended to use the expanded [[..]]
test for string/file tests, and ((..))
for numerical tests. Avoid using the old [..]
test unless you specifically need POSIX-style portability.
d. Re your previous post, The if
statement isn't really necessary here, but it does have the minor benefit of slightly lightening the processing load, as the external mkdir
command will only be run if needed.
e. Clean, consistent
formatting makes code readable and more easily debuggable. Be liberal with whitespace; indent all your sub-commands evenly and separate logical sections with empty lines. Never just line everything up on the left edge.
Scripting With Style
grep -lr -e '$OLDTHEMENAME' * | xargs sed -i 's|$OLDTHEMENAME|$THEMENAMELC|g'
a. Quoting again.
b. When processing filenames, particularly with xargs
, you should really use null separators
. Although this depends on your tools having support for them.
Assuming the gnu versions of the tools, I believe you can do this:
grep -lr -Z -e '$OLDTHEMENAME' * | xargs -0 sed -i "s|$OLDTHEMENAME|$THEMENAMELC|g"
If you aren't using the gnu toolset, check your documentation to see if null separators are supported.
I recommend always supplying an explicit exit value, rather than relying on the default, which is the exit status of the final command run.