Tag Archives: claws

Reviewing patches

I always struggled at reviewing code.

Specially when the code to be reviewed is in reality a patch inlined in some e-mail… I hate monospaced fonts in my e-mail reader, and with all the context switches I got in my daily work, I simply can’t concentrate properly in order to follow what’s been proposed with that one patch out of many, in that long long patch series.

In the past, I used to apply them manually, then go over the code using Source Navigator and later cscope.

I still miss the ability to jump between symbol definition and use that cscope does the best, but I have a much more streamlined way of reviewing patches today, thanks to git, meld, and claws-mail.

The first thing is about git. Nowadays I use git in every coding project I use – even if the upstream project is not using git as SCM itself (I simply create a local repository and import). And this is not only for making reviewing patches easier, but all sorts of things, like fast branching and merging, easy cherry-picking, rebasing, commit amending, modern utilities et al. It’s really the 21st century version control system.

The second thing is meld. Meld is one good example of an intuitive interface that doesn’t get in the way. It can compare, merge and edit files (up to 3-way merge if needed). Supports all the major SCMs such as git, hg, cvs and svn (although I can’t find a reason why would anyone still use the last two, at least locally).

Meld side-by-side diff
Meld side-by-side diff

The forth thing, and where actually everything makes sense, is Claws-mail, which has the very useful (and unique?) ability to create custom actions to process messages.

Claws-Mail Actions
Claws-Mail Actions

Guess what happens when you combine Claws-Mail’s actions with a script that uses git and Meld? A very point-and-click way of reviewing patches:

Claws-Mail, git and Meld in action
Claws-Mail, git and Meld in action

The trick is in configuring an action in Claws-Mail that opens a terminal and calls a script. The script uses git-am to apply the patch contained within the selected mail message to some branch in your local git repository. After applying, it calls git-difftool to show the differences. git-difftool then calls any diff tool you might like (my suggestion stays with Meld).

I’m attaching the script for reference below:

#!/bin/sh
## git-review-step
## (C) Copyright 2010 Klaus Heinrich Kiwi
## Licensed under CreativeCommons Attribution-ShareAlike 3.0 Unsupported
## http://creativecommons.org/licenses/by-sa/3.0/ for more info.
 
## dirname is where the git tree is located.
dirname=$HOME/sandbox/ock/sourceforge-git/opencryptoki
 
if [ "$#" -lt 1 ]; then
  echo "Invalid number of parameters"
  echo "usage: $(basename $0) <patch1> [patch2] [patch3] [...]"
  exit 1
fi
 
messages=($@)
 
cd $dirname
oldbranch=`git branch | grep -e '^* ' | cut -d " " -f 2`
 
# Save any uncommitted changes in the working dir or index
if git stash | grep HEAD; then
  savedchanges="yes"
fi
 
function restore() {
  echo "Reverting to original branch..."
  git checkout --force $oldbranch
  if [ -n "$savedchanges" ]; then
    echo "Restoring un-committed changes..."
    git stash pop
  fi
}
 
# Get branch to apply to
git branch
echo "Select branch to apply patches:"
echo "  Enter \"<branchname>\" to apply to an existing branch"
echo "  Enter \"<newname> [origref]\" to create a new branch from \"origref\""
echo "    reference (use current branch and HEAD if left blank)"
read -p "Apply patch(es) to branch (default is current):" -e -i $oldbranch newbranch origbranch
 
if [ -n "$newbranch" ]; then
  if git branch | grep -e "\b${newbranch}$"; then
    echo "Applying to existing branch \"$newbranch\""
    # Checkout
    if ! git checkout $newbranch; then
      echo "Error checkout out \"$newbranch\" - Aborting"
      restore
      read -p "Press Enter to continue"
      exit 1
    fi
  else
    if [ -n "$origbranch" ]; then
      echo "Applying to new branch \"$newbranch\" created from \"$origbranch\" branch..."
    else
      echo "Applying to new branch \"$newbranch\" created from \"$oldbranch\" branch..."
    fi
    if ! git checkout -b $newbranch $origbranch; then
      echo "Error creating \"$newbranch\" from \"$oldbranch\" - Aborting"
      restore
      read -p "Press Enter to continue"
      exit 1
    fi  # if ! git checkout ...
 
  fi    # if `git branch | grep ...
 
fi      # if [ -n $newbranch ...
 
# Apply patches to working dir using git-apply
amparams="--whitespace=error-all"
while ! git am $amparams ${messages[@]}; do
  git am --abort
  echo "git-am failed. Retry (the whole chunk) with additional parameters?"
  read -p "git-am parameters (empty aborts):" -e -i $amparams amparams
  if [ -z "$amparams" ]; then
    echo "Aborting..."
    restore
    read -p "Press Enter to continue"
    exit 1
  fi
done
 
for (( i=${#messages[@]}; i > 0; i-- )); do
  PAGER='' git log --stat HEAD~${i}..HEAD~$((i-1))
  if git diff --check HEAD~${i}..HEAD~$((i-1)); then
    echo "WARNING: Commit introduces whitespace or indenting errors"
  fi
  git difftool HEAD~${i}..HEAD~$((i-1))
done
 
echo "Restoring working tree to original state"
restore
read -p "Press Enter to continue"

Creative Commons License
git-review-step by Klaus Heinrich Kiwi is licensed under a Creative Commons Attribution-ShareAlike 3.0 Unported License.
Based on a work at blog.klauskiwi.com.