up vote 3 down vote favorite

I have a function here and wondered how well this can be refactored.

I currently have this:

status=`git status 2>&1 | tee`
dirty=`echo -n "${status}" 2> /dev/null | grep "modified:" &> /dev/null; echo "$?"`
untracked=`echo -n "${status}" 2> /dev/null | grep "Untracked files" &> /dev/null; echo "$?"`
ahead=`echo -n "${status}" 2> /dev/null | grep "Your branch is ahead of" &> /dev/null; echo "$?"`
newfile=`echo -n "${status}" 2> /dev/null | grep "new file:" &> /dev/null; echo "$?"`
renamed=`echo -n "${status}" 2> /dev/null | grep "renamed:" &> /dev/null; echo "$?"`
deleted=`echo -n "${status}" 2> /dev/null | grep "deleted:" &> /dev/null; echo "$?"`
bits=''
if [ "${renamed}" == "0" ]; then
  bits=">${bits}"
fi
if [ "${ahead}" == "0" ]; then
  bits="*${bits}"
fi
if [ "${newfile}" == "0" ]; then
  bits="+${bits}"
fi
if [ "${untracked}" == "0" ]; then
  bits="?${bits}"
fi
if [ "${deleted}" == "0" ]; then
  bits="x${bits}"
fi
if [ "${dirty}" == "0" ]; then
  bits="!${bits}"
fi
if [ ! "${bits}" == "" ]; then
  echo " ${bits}"
else
  echo ""
fi

I have played with it a little bit but I'm not an expert with bash so refactoring this is a little tricky, here is my attempt:

gitstatus=( 
  "renamed;renamed:;>"
  "dirty;modified:;!"
  "untracked;Untracked files;?"
  "ahead;Your branch is ahead of;*"
  "newfile;new file:;+"
  "deleted;deleted:;x"
)

bits=''

for status in "${gitstatus[@]}"; do

  value=(${status//;/ })
  check=`echo -n "${value[0]}" 2> /dev/null | grep "${value[1]}" &> /dev/null; echo "$?"`

  if [ "${check}" == "0" ]; then
    bits="${value[2]}${bits}"
  fi

done

if [ ! "${bits}" == "" ]; then
  echo " ${bits}"
else
  echo ""
fi
share|improve this question
6  
have you considered using --porcelain option, which is intended to give output that's easier to parse? – Vogel612 Jan 22 '16 at 21:29
    
@Vogel612 yes that is something I have used before but didn't cross my mind here, its something I will look into. – Oliver Atkinson Jan 22 '16 at 21:39
up vote 3 down vote accepted

Porcelain format

As @Vogel612 noted, the --porcelain format is the right one to use for scripting:

The porcelain format is similar to the short format, but is guaranteed not to change in a backwards-incompatible way between Git versions or based on user configuration. This makes it ideal for parsing by scripts.

For the purposes of reviewing your code, though, I'll stick with the human format.

Redirection

There seems to be unusual concern placed on the handling of the standard error streams. For the most part, you shouldn't need to worry about standard error that much. As a rule of thumb, standard error contains out-of-band error messages that aren't really part of the output, and therefore aren't worth parsing unless you are interested in capturing those messages. It may or may not be a good idea to redirect it to /dev/null.

As an example of that concern, I see git status 2>&1. What sorts of messages might be in git's standard error. One example would be

fatal: Not a git repository (or any of the parent directories): .git

Is that something that we would be interested in parsing? Not really. Is that a message that we want to make sure that the user never sees? Maybe. Therefore, standard error should either be left alone or redirected to /dev/null; lumping it together with standard output produces little benefit.

The standard error redirection in echo -n "${status}" 2> /dev/null is just superfluous. Echoing a string should never produce anything on standard error. As for grep … &> /dev/null, consider doing grep -q instead.

I don't understand why you need to | tee.

One pass, no buffering

Instead of buffering the $status and doing many command substitutions with echo -n "${status}" … | grep …, I suggest doing it all in one pass in pure Bash.

git status | (
    unset dirty deleted untracked newfile ahead renamed
    while read line ; do
        case "$line" in
          *modified:*)                      dirty='!' ; ;;
          *deleted:*)                       deleted='x' ; ;;
          *'Untracked files:')              untracked='?' ; ;;
          *'new file:'*)                    newfile='+' ; ;;
          *'Your branch is ahead of '*)     ahead='*' ; ;;
          *renamed:*)                       renamed='>' ; ;;
        esac
    done
    bits="$dirty$deleted$untracked$newfile$ahead$renamed"
    [ -n "$bits" ] && echo " $bits" || echo
)
share|improve this answer
    
Thanks - good review and explanation. It has been some inherited code which I wanted to improve and understand. – Oliver Atkinson Jan 23 '16 at 9:52
    
gist.github.com/ollieatkinson/c649d8c85aef714acd63 – Oliver Atkinson Jan 23 '16 at 10:37
    
If you're using this code to produce a Bash prompt, then git status 2>/dev/null makes sense, because you never want to show any errors to the user. You also don't need the || echo at the end. – 200_success♦ Jan 23 '16 at 18:38

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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