4
\$\begingroup\$

I have a GitHub repository containing all my dotfiles for bash, tmux and neovim. I'm using a Mac and whenever I factory reset it I like to just pull repository from GitHub and link all the dotfiles to the one in the repo.

Now I wrote a script to automatically create these symlinks and handle possible errors on the way.

This is what I came up with:

#!/usr/bin/env bash

RED=`tput setaf 1`
GREEN=`tput setaf 2`
RESET=`tput sgr0`

custom_link() {
  file_loc="$1"
  symlink_loc="$2"

  # Check whether original file is valid
  if [[ ! -e "$file_loc" ]]; then
    echo "${RED}WARNING${RESET}: Failed to link $symlink_loc to $file_loc"
    echo "         $file_loc does not exist"
    return 1
  fi

  # Check whether symlink already exists
  if [[ -e "$symlink_loc" ]]; then
    if [[ -L "$symlink_loc" ]]; then
      if [[ "$(readlink $symlink_loc)" == "$file_loc" ]]; then echo "--> $symlink_loc -> $file_loc ${GREEN}exists${RESET}."; return 0; fi

      current_dest=$(readlink "$symlink_loc")
      echo "It seems like $symlink_loc already is symlink to $current_dest."
      read -p "Do you want to replace it? [Y/N] " -n 1 -r
      if [[ $REPLY =~ ^[Yy]$ ]]
      then
        echo
        echo "Removing current symlink..."
        unlink  $symlink_loc
        ln -s "$file_loc" "$symlink_loc"
        echo "Created a symlink $symlink_loc -> $file_loc"
        return 0
      fi
      echo
      return 2
    else
      echo "It seems like $symlink_loc already exists."
      read -p "Do you want to [d]elete, [m]ove (-> $HOME/.other/) or [k]eep $symlink_loc? " -n 1 -r
      if [[ $REPLY =~ ^[Dd]$ ]]
      then
        printf "\nDeleting $symlink_loc..."
        rm -rf "$symlink_loc"
        ln -s "$file_loc" "$symlink_loc"
        echo "Created a symlink $symlink_loc -> $file_loc"
        return 0
      elif [[ $REPLY =~ ^[Mm]$ ]]; then
        printf "\nMoving $symlink_loc to ~$HOME/.other..."
        mkdir "$HOME/.other"
        mv "$symlink_loc" "$HOME/.other"
        ln -s "$file_loc" "$symlink_loc"
        echo "Created a symlink $symlink_loc -> $file_loc"
        return 0
      else
        printf "\nKeeping $symlink_loc...\n"
        return 2
      fi
    fi
  fi

  symlink_dir=$(dirname "$symlink_loc")
  if ! mkdir -p "$symlink_dir" ; then
   echo "${RED}WARNING${RESET}: Failed to link $symlink_loc to $file_loc"
   echo "         Could not create folder $symlink_dir/"
   return 1
  fi

  ln -s "$file_loc" "$symlink_loc"
  echo "Created a symlink $symlink_loc -> $file_loc"
  return 0
}

custom_link "$HOME/.dotfiles/bash/.bashrc" "$HOME/.bash_profile"
custom_link "$HOME/.dotfiles/bash/.bashrc" "$HOME/.bashrc"
custom_link "$HOME/.dotfiles/nvim/init.vim" "$HOME/.config/nvim/init.vim"
custom_link "$HOME/.dotfiles/tmux/.tmux.conf" "$HOME/.tmux.conf"

It was my very first time writing a bash script and I just googled, whenever I had a specific question.

I choose the return codes as follows: - 0 for successful creation - 1 for failure - 2 for cases, when the link was not created, because the user decided so.

My questions are:

  • How is my general bash programming style? Any important idioms I missed?
  • The script contains a lot of duplicate code (especially the three lines: ln -s "$file_loc" "$symlink_loc", echo "Created a symlink $symlink_loc -> $file_loc" and return 0. How can I reduce the number of duplicates?
  • How could I implement more successful return values?
  • I handles a lot of cases (the file already exists, etc.). Did I miss any important cases?
  • Currently the function custom_link displays a lot of text. How could I outsource this echoing to the outside of the function? The function should do the bare minimum (maybe connected to the question about useful return codes).
\$\endgroup\$
6
\$\begingroup\$

I noticed that some of your questions have similar aspects, so I clustered them to provide a single answer:

"How is my general bash programming style?"
"How can I reduce the number of duplicates?"
"How could I outsource this echoing to the outside of the function?"

Shell script is ugly compared to other scripting languages, so we need to make an extra effort to keep it readable.

So here are some suggestions to improve the readability of your script:

  • Check your script with ShellCheck for insights on best practices and idioms.

  • Extract those file checks to functions with meaningful names, so that the reader won't need to remember (or lookup) what each flag means:

    if [[ -e "$symlink_loc" ]]; then
        if [[ -L "$symlink_loc" ]]; then
    

    could become:

    if file_exists "$symlink_loc"; then
        if is_symlink "$symlink_loc"; then
    


  • Hide low level details that are polluting the custom_link function:

    custom_link() {
        # 1. checking if original file is valid
            # 1.1 if not valid, print error message and return 1
    
        # 2. handling existing file where symlink should be placed
            # 2.1 if is already existing symlink
                # 2.1.1 if symlink already points to file, print success and return 0
                # 2.1.2 otherwise, ask user if symlink should be replaced
                    # 2.1.2.1 if yes, replace it
    
            # 2.2 if is a regular file
                # 2.2.1 prompt user to delete, move or keep file
                    # 2.2.1.1 if delete, delete file and place symlink
                    # 2.2.1.2 if move, create dir $HOME/.other, move file there and place symlink
                    # 2.2.1.3 if keep, print "keeping the file"
    
        # 3. creating symlink
            # 3.1 extract dirname of symlink
            # 3.2 try to create extracted dir
                # 3.2.1 if fails, print error and return 1
            # 3.3 create symlink, print success and return 0
    }
    

    It should be easy for the reader of custom_link to understand what is going on, on a high level:

    custom_link() {
        file_loc="$1"
        symlink_loc="$2"
    
        if ! file_exists "$file_loc"
        then
            return invalid_file_error "$file_loc" "$symlink_loc"
        fi
    
        if file_exists "$symlink_loc"
        then
            return handle_existing_file "$file_loc" "$symlink_loc"
        fi
    
        create_new_symlink "$file_loc" "$symlink_loc"
    }
    

    And then you can hide lower level details of each step on their own dedicated functions. (You can apply this recursively, until the functions are very small and obvious)
    Here are some examples:

    file_exists() { test -e "$1"; }
    
    invalid_file_error() {
        file_loc="$1"
        symlink_loc="$2"
    
        echo "${RED}WARNING${RESET}: Failed to link $symlink_loc to $file_loc"
        echo "         $file_loc does not exist"
        return 1
    }
    
    handle_existing_file() {
        file_loc="$1"
        symlink_loc="$2"
    
        if is_symlink "$symlink_loc"
        then
            return handle_existing_symlink "$file_loc" "$symlink_loc"
        else
            return handle_existing_regular_file "$file_loc" "$symlink_loc"
        fi
    }
    


"Any important idioms I missed?"
"How could I implement more successful return values?"

The convention is to return 0 for success and anything else for errors.

So you got it right when you are returning 0 (symlink already there pointing to expected file) and 1 (file doesn't exist).

But when you prompt the user on which actions should be taken, you are successfully executing what was requested, but returning a 2.

If you think from the user's perspective, the exit code should be 0, since the script did exactly what the user wanted.


"Did I miss any important cases?"

The user can choose to move a file to $HOME/.other, but what should happen if there is already a file there (on $HOME/.other) with the same name, maybe from a previous run?

\$\endgroup\$
  • \$\begingroup\$ Thank you really much for your answer! I'll probably accept it in the next few hours. \$\endgroup\$ – user7802048 Dec 3 '17 at 13:38

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.