-3

I am new to bash. Can someone please point me to the error in the code.

for x in `ls`;do `if(echo $x|grep "-2"); then rm $x; fi`; done;

I am basically trying to remove all the files ending with "-2.jpg"

tallandroid
  • 726
  • 1
  • 7
  • 8

2 Answers2

1

Don't use command substitution (backquotes) inside your for block. Command substitution is only used for getting the output to be used for assigning to a variable or for evaluating as another command.

`if(echo $x|grep "-2"); then echo 'yo'; fi`

You also need to add a space between if and the expression that follows it:

if (echo $x|grep "-2"); ...

Command substitution is also not necessary:

if echo $x | grep "-2"

You can also use -q to prevent messages being shown from grep:

if echo $x | grep -q "-2"

You also need to use -e to preven -2 from being interpretted as a wrong option to grep:

if echo $x | grep -q -e "-2"

Some of your variables should also be placed inside double quotes to prevent word splitting:

if echo "$x" | grep -q -e "-2"

Prefer $() over backticks when doing command substitution as well:

for x in $(ls);do ...

Finally the better form:

for x in $(ls); do if echo "$x" | grep -q -e "-2"; then echo 'yo'; fi; done

Additional recommendations:

Better not get values through word splitting as it's also subject to pathname expansion and unexpected changes may occur. Use while read loop and process substitution instead. With ls, use -1 as well to show entries line by line:

while IFS= read -r x; do if echo "$x" | grep -q -e "-2"; then echo 'yo'; fi; done < <(ls -1)

When doing command substitution or process substitution, if the last command is a call to an external binary, add exec to prevent unnecessary fork:

$(exec ls -1)
<(exec ls -1)

You can also just use comparisons over using grep:

if [[ $x == *-2* ]]; then
konsolebox
  • 72,135
  • 12
  • 99
  • 105
0

You have to remove the backquotes for the do part, otherwise it means that you want to execute the filename. Also consider this alternative:

for x in `find -name "*-2.jpg"`; do echo $x; echo "yo"; done
enrico.bacis
  • 30,497
  • 10
  • 86
  • 115
  • `find != ls`, and this is going to suffer from largely the same issues as `for whatever in $(ls)` does, and more since you'll likely get more files which will take forever to buffer. See [this answer](http://stackoverflow.com/a/7039579/3076724) for a better syntax to iterate `find` results (though no loop is really needed here) – Reinstate Monica Please Aug 17 '14 at 18:27
  • @BroSlow Thanks for the link and of course you're right, but the OP said he needs to find files with that suffix, so `find` should be ok. For the syntax, it was to be closer to the OP one, to be more effective, and for the issues, the OP code doesn't work because of one issue that my solution does not present. Every command could have its issues, but we should address only the one stated in the question. – enrico.bacis Aug 17 '14 at 18:31
  • Addressing `only the one stated in the question` is ok, but it's not going to be particularly helpful when the user runs into other issues from using an error-prone syntax, or when other users see the answer and assume it's an example of good syntax. And `find` can obviously get the same results as the user is asking about in the question, but not without arguments, e.g. something like `find -mindepth 1 -maxdepth 1 \! -name '.*'` would essentially mimick `ls`. – Reinstate Monica Please Aug 17 '14 at 18:51