3
\$\begingroup\$

The objective of this code is to create a command that move files and leaves links in their place. I'm just a beginner in shell scripting and it would be quite useful to hear your opinions on how to improve my code.

I already used https://www.shellcheck.net/ to check my code.

Questions

I change directory several times because I was creating broken links with relative paths. Is there a better way to do this?

Is there a better way to get the path of the files than to use cut with wc and using cd and pwd?

mvln.sh

#!/bin/bash
if [ "$#" -lt 2 ];then 
    echo -e "mvln: Move file and leave a link in its place\\nUse: mvln [TARGET] [DIRECTORY]";
    exit 1;
fi

pathTo=$(cut -d ' ' -f "$#" <<< "$*"); 
if ! [ -d "$pathTo" ]; then
    echo "mvln: $pathTo is not a directory";
    exit 1;
fi

pathOriginal=$(pwd);
cd "$pathTo" || exit 1;
pathTo=$(pwd);
cd "$pathOriginal" || exit 1;

files=$(cut -d ' ' -f -$(("$#" - 1)) <<< "$*");

for i in $files;
do 
    if [ "$i" -ef "$pathTo" ]; then
        echo "mvln: '$pathFrom' and '$pathTo' are the same file";
        continue;
    fi
    wordCount=$(tr -t '/' ' ' <<< "$i" | wc -w);
    pathFrom=$(cut -d '/' -f -$((wordCount - 1)) <<< "$i");
    file=$(cut -d '/' -f "$wordCount" <<< "$i");
    mv "$i" "$pathTo" || continue;
    cd "$pathFrom" || exit 1;
    ln -s "$pathTo/$file" ".";
    cd "$pathOriginal" || exit 1
done;
exit 0;

Creating test files

mkdir ~/Documentos ~/Scripts ~/Scripts/test ~/Scripts/test/ln
touch ~/arqHome.txt ~/Documentos/arqDoc.txt ~/Scripts/arqScripts.txt ~/Scripts/test/arq1.txt ~/Scripts/test/arq2.txt ~/Scripts/test/arq3.txt ~/Scripts/test/arq4.txt ~/Scripts/test/arq5.txt

Examples of how the command works:

  • Example1

    Move all files from 1 place

    bash mvln.sh ~/Scripts/test/arq* ~/Scripts/test/ln
    
  • Example2

    Move files from several places

    bash mvln.sh ~/arqHome.txt ~/Documentos/arqDoc.txt ~/Scripts/arqScripts.txt ~/Scripts/test/arq* ~/Scripts/test/ln
    
\$\endgroup\$
  • \$\begingroup\$ Version 2 added! \$\endgroup\$ – Vencato Apr 1 '18 at 4:50
  • \$\begingroup\$ I have rolled back your last edit. Please don't change or add to the code in your question after you have received answers. See What should I do when someone answers my question? Thank you. \$\endgroup\$ – Phrancis Apr 1 '18 at 5:03
2
\$\begingroup\$

Instead of temporarily changing directories, you can call realpath to get the absolute path to a directory. There is also readlink which does similar job.


To generate the files array:

files=("$@"); unset "files[-1]"

The rest looks okay to me. However, keep your usage of ; consistent. If you are using it, put them in each statement; otherwise skip it altogether.

\$\endgroup\$
  • \$\begingroup\$ Thanks! That unset files[-1] tip seems interenting! \$\endgroup\$ – Vencato Apr 1 '18 at 4:47
  • \$\begingroup\$ what is the diference from files=("$@"); and files=$("$@");? \$\endgroup\$ – Vencato Apr 7 '18 at 14:57
1
\$\begingroup\$

Turn on more error-checking

I always find it useful to error out on undefined variables or command failures in Bash scripts:

set -eu

Omit unnecessary statement separators

Most of the ; (at end of line) are not needed, and just add visual clutter.

Options to echo are not portable

Instead of echo -e, consider using multiple echo commands, printf, or $'' quoting instead.

Error messages should go to the error stream

Just add >&2 to the commands that print error/warning messages. I like to define a short function for this:

die() {
    printf '%s\n' "$@" >&2
    exit 1
}

We can use $0 for the program name (as used) rather than hard-coding a name.

Consider ln -r to create relative symlinks

Instead of changing working directory as you go (which, as you have found, is inconvenient and confusing), take advantage of your ln implementation. In particular the -r or --relative option does what you want (though you might want to avoid this if the user supplied an absolute pathname).

Use bash array variables to hold lists of names

The current handling of arguments doesn't work very well for filenames containing newlines or spaces. That can be improved using array variables.

Use variable-expansion to transform filenames

wordCount is unreliable for names containing spaces. The Bash idiom to remove everything up to and including the last / from a variable called i is ${i##*/}.

Let mv and ln check for errors

Instead of testing whether target file is the same as source, just rely on mv to provide its own error message, and use the return value to see whether it succeeded.

Return non-zero if any of the operations failed

Only return a true (zero) code if everything the user asked for has been done.


Modified version

#!/bin/bash

die() {
    printf '%s\n' "$@" >&2
    exit 1
}

test "$#" -ge 2 \
    || die "$0: Move file and leave a link in its place" \
           "Usage: $0 TARGET... DIRECTORY" \
           "Copy each TARGET into DIRECTORY, and replace it with a symlink."

src=("$@")

dest="${src[-1]}"
test -d "$dest" || die "$0: $dest is not a directory"

unset src[-1]

result=true

for i in "${src[@]}"
do
    mv -t "$dest" "$i" && ln -srT "$dest/${i##*/}" "$i" || result=false
done

$result
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.