3
\$\begingroup\$

This is the bash script responsible for changing the directories via textual tags. (See here the entire project.)

dt_script

~/.dt/dt $@ > ~/.dt/dt_tmp
COMMAND_TYPE=$(head -n 1 ~/.dt/dt_tmp)
if [ $# -eq 0 ]; then
    NEXT_PATH=$(~/.dt/dt prev | tail -n 1) # Let dt return pwd if no prev tag.
    ~/.dt/dt --update-prev $(pwd)
    if [ "$COMMAND_TYPE" == "message" ]; then
        tail -n 1 ~/.dt/dt_tmp
    else
        cd $NEXT_PATH
    fi
else
    if [ "$COMMAND_TYPE" == "switch_directory" ]; then
        NEXT_PATH=$(tail -n 1 ~/.dt/dt_tmp)
        ~/.dt/dt --update-prev $(pwd)
        cd $NEXT_PATH
    elif [ "$COMMAND_TYPE" == "show_tag_entry_list" ]; then
        tail -n +2 ~/.dt/dt_tmp  
    elif [ "$COMMAND_TYPE" == "message" ]; then
        tail -n 1 ~/.dt/dt_tmp
        #echo $(cut -d " " -f 2)
    else
        echo "Unknown command: $COMMAND_TYPE"
    fi
fi
rm ~/.dt/dt_tmp

Any further suggestions for improvement are welcomed.

\$\endgroup\$
3
\$\begingroup\$

Some suggestions:

  • Use More Quotes™
    • $@ should be double quoted to pass the parameters without word splitting.
    • Any variable use should be quoted. It is very rare that word splitting is actually what you want, and it causes sometimes very subtle bugs.
    • Command substitution should be quoted.
  • Use good bashisms such as [[ instead of [.
  • Local variables should be lowercase to distinguish them from system variables, which are uppercase.
  • There is no need for semicolons in Bash scripts. This may be more contentious, but I generally write like this:

    if [[ "$variable" = 'value' ]]
    then
        …
    

    Apropos: == to compare strings is a bit of a historical accident. The original operator is just =, but == is probably not going away.

  • $(pwd) can be simplified to . - it also means the current directory.
  • The home directory should not be used for temporary file storage (unless that is actually your configured temporary directory). In general you should use something like working_dir="$(mktemp --directory)" (or -d if you don't have GNU coreutils mktemp). This has a few advantages:
    • It means your home directory isn't going to fill up with temporary files which are never cleaned up (but more about that below).
    • It is often a memory mapped filesystem, so it may be much faster than your home directory.
  • Long if/elif sections testing the value of a single variable can usually be easier written as a case block.
  • head does not fail if run on an empty file:

    $ touch foo
    $ head foo
    $ echo $?
    0
    

    So your first test could fail for the wrong reason. You might want to [[ -s "$path" ]] to check if a file is empty.

  • The file name "dt_tmp" doesn't tell me anything about what it contains or what it's used for - in general I find that adding the project name to anything within the project is redundant, and marking something as temporary is not particularly helpful to know what it is, unless it really can contain anything. Is there a better name you can give it? It looks like it's some sort of command queue, maybe?
\$\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.