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:

## 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.
if [ "$#" -lt 1 ]; then
  echo "Invalid number of parameters"
  echo "usage: $(basename $0) <patch1> [patch2] [patch3] [...]"
  exit 1
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
function restore() {
  echo "Reverting to original branch..."
  git checkout --force $oldbranch
  if [ -n "$savedchanges" ]; then
    echo "Restoring un-committed changes..."
    git stash pop
# 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"
      read -p "Press Enter to continue"
      exit 1
    if [ -n "$origbranch" ]; then
      echo "Applying to new branch \"$newbranch\" created from \"$origbranch\" branch..."
      echo "Applying to new branch \"$newbranch\" created from \"$oldbranch\" branch..."
    if ! git checkout -b $newbranch $origbranch; then
      echo "Error creating \"$newbranch\" from \"$oldbranch\" - Aborting"
      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
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..."
    read -p "Press Enter to continue"
    exit 1
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"
  git difftool HEAD~${i}..HEAD~$((i-1))
echo "Restoring working tree to original state"
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.