2
\$\begingroup\$

For the first time ever I decided to write a bash script. It works fine, but I'm feeling that I didn't write the most efficient code in order for future extension of the script. I also don't really get a good feeling on the look of the code. Any suggestions on how to improve the script?

Purpose of the script: To pull a vim configuration located on github and then copy the files to the home directory and create some directories needed by the vim configuration.

dirs=( "~/.vim/undo" "~/.vim/swap" "~/.vim/backup" )
function error_exit
{
  printf '[failed\n]'
  exit 1
}

printf '   vim_config by Roel0                \n'
printf '   version 1.0                       \n'

printf 'Updating vim configuration ... '
if !(git pull > /dev/null;) then
  error_exit
fi
printf '[OK]\n'

printf 'Installing vim configuration ... '
if !(cp -r ./.vim* ~/) then
  error_exit
fi
printf '[OK]\n'

printf 'Creating configured directories ... '
for i in "${dirs[@]}"
do
  if !(mkdir -p $i) then
    error_exit
  fi
done
printf '[OK]\n'
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! Please tell us what the intended purpose of the code is (and also retitle the post accordingly). \$\endgroup\$ – 200_success Jan 25 '16 at 8:39
  • \$\begingroup\$ Does this actually work? I don't think !(cmd) is valid syntax. And since ~ is not expanded within double-quotes, I doubt the script will do what you want. Have you tested this well? Are you sure it works? \$\endgroup\$ – janos Jan 27 '16 at 13:19
1
\$\begingroup\$

Shebang

You should declare what shell you require to make porting the script easy and not a nightmare of commands not being available and syntactical differences leading to bugs.

#!/bin/bash

Pull conflicts

The script does not account for problems when pull does funny things like conflicting. Also to prevent changes to the executable run through PATH modifications (whether intentional or not) you may want to consider using the absolute executable path.

Looping

I'd personally prefer the simpler loop instead of the string looping you do there.

for i in $dirs
do
  if !(mkdir -p $i) then
    error_exit
  fi
done

at least on my zsh this works as intended.

\$\endgroup\$
  • \$\begingroup\$ What is "string looping"? He's looping over an array correctly, in a way that would handle spaces in element names. for i in $dirs weakens that. Does !(mkdir ...) really work for you? It doesn't work for me in bash or zsh. (Left a comment for OP too.) \$\endgroup\$ – janos Jan 27 '16 at 13:22

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.