Shell script with absolute path and control errors

Question

I was doing this little script in which the first argument must be a path to an existing directory and the second any other thing. Each object in the path indicated in the first argument must be renamed so that the new name is the original that was added as a prefix to the character string passed as the second argument. Example, for the string "hello", the object OBJECT1 is renamed hello.OBJECT1 and so on

Additionally, if an object with the new name is already present, a message is shown by a standard error output and the operation is not carried out continuing with the next object.

I have the following done:

#! /bin/bash

if [ "$#" != 2 ]; then
        exit 1
else
        echo "$2"
        if [ -d "$1" ]; then
                echo "directory"
            for i in $(ls "$1")
                do
                for j in $(ls "$1")
                   do
                      echo "$i"
                      if [ "$j" = "$2"."$i" ]; then
                         exit 1
                      else
                         mv -v "$i" "$2"."$i"
                         echo "$2"."$i"
                      fi
                done
            done

        else
           echo "no"
        fi
fi

I am having problems if I run the script from another file other than the one I want to do it, for example if I am in /home/pp and I want the changes to be made in /home/pp/rr, since that is the only way It does in the current.

I tried to change the ls to catch the whole route with ls -R | sed "s;^;pwd;" but the route catches me badly.

Using find you can't because it puts me in front of the path and doesn't leave the file

Then another question, to verify that that object that is going to create new is not inside, when doing it with two for I get bash errors for all files and not just for coincidences

I'm starting with this scripting, so it has to be a very simple solution thing

[Don't use `ls`](http://mywiki.wooledge.org/ParsingLs) and [quote your variables.](https://stackoverflow.com/questions/10067266/when-to-wrap-quotes-around-a-shell-variable) Once your script survives http://shellcheck.net try `man basename`. — tripleee, Dec 30 '19 at 15:02
I guess "the route catches me badly" means you want to replace the path, but the input does not contain the full path? Not using `ls` fixes that trivially, of course; but you probably want to do `ls -R "$1" | sed "s;^$1;;"` instead of operating on the full value of `pwd` — tripleee, Dec 31 '19 at 09:33

score 1 · Answer 1 · edited Dec 31 '19 at 09:22

An obvious answer to your question would be to put a cd "$2 in the script to make it work. However, there are some opportunities in this script for improvement.

#! /bin/bash

if [ "$#" != 2 ]; then

You might put an error message here, for example, echo "Usage: $0 dir prefix" or even a more elaborate help text.

    exit 1
else
    echo $2

Please quote, as in echo "$2".

    if [ -d $1 ]; then

Here, the quotes are important. Suppose that your directory name has a space in it; then this if would fail with bash: [: a: binary operator expected. So, put quotes around the $1: if [ -d "$1" ]; then

        echo "directory"

This is where you could insert the cd "$1".

        for i in $(ls $1)
            do

It is almost always a bad idea to parse the output of ls. Once again, this for-loop will fail if a file name has a space in it. A possible improvement would be for i in "$1"/* ; do.

            for j in $(ls $1)
               do
                  echo $i
                  if [ $j = $2.$i ]; then
                     exit 1
                  else

The logic of this section seems to be: if a file with the prefix exists, then exit instead of overwriting. It is always a good idea to tell why the script fails; an echo before the exit 1 will be helpful.

The question is why you use the second loop? a simple if [ -f "$2.$i ] ; then would do the same, but without the second loop. And it will therefore be faster.

                     mv -v $i $2.$i
                     echo $2.$i

Once again: use quotes!

                  fi
            done
        done

    else
       echo "no"
    fi
fi

So, with all the remarks, you should be able to improve your script. As tripleee said in his comment, running shellcheck would have provided you with most of the comment above. But he also mentioned basename, which would be useful here.

With all that, this is how I would do it. Some changes you will probably only appreciate in a few months time when you need some changes to the script and try to remember what the logic was that you had in the past.

#!/bin/bash

if [ "$#" != 2 ]; then
    echo "Usage: $0 directory prefix" >&2
    echo "Put a prefix to all the files in a directory." >&2
    exit 1
else
    directory="$1"
    prefix="$2"
    if [ -d "$directory" ]; then
        for f in "$directory"/* ; do
            base=$(basename "$f")
            if [ -f "Sdirectory/$prefix.$base" ] ; then
                echo "This would overwrite $prefix.$base; exiting" >&2
                exit 1
            else
                mv -v "$directory/$base" "$directory/$prefix.$base"
            fi
       done
    else
       echo "$directory is not a directory" >&2
    fi
fi

Shell script with absolute path and control errors

1 Answers1