Re: [PATCH 2/3] rebase-i: slight internal improvements

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Jun 20, 2008, 07:16 UTC
Message-ID: <485B595B.80608@viscovery.net>
In-Reply-To: <1213929935-15093-3-git-send-email-s-beyer@gmx.net>

Stephan Beyer schrieb:

> Add commit_message function to get the commit message
> from a commit and other slight internal improvements.

If by "other slight improvements" you mean ...

Show 11 quoted lines

>  mark_action_done () {
> -	sed -e 1q < "$TODO" >> "$DONE"
> -	sed -e 1d < "$TODO" >> "$TODO".new
> -	mv -f "$TODO".new "$TODO"
> -	count=$(grep -c '^[^#]' < "$DONE")
> -	total=$(($count+$(grep -c '^[^#]' < "$TODO")))
> +	sed -e 1q "$TODO" >>"$DONE"
> +	sed -e 1d "$TODO" >>"$TODO.new"
> +	mv -f "$TODO.new" "$TODO"
> +	count="$(grep -c '^[^#]' "$DONE")"
> +	total="$(expr "$count" + "$(grep -c '^[^#]' "$TODO")")"

... this ...

>  has_action () {
> -	grep '^[^#]' "$1" >/dev/null
> +	grep -q '^[^#]' "$1"

... and this, etc, then they are not improvements. They make the script less portable: There are 'grep's that don't have -q, others write the file name in front of the count, and I _think_ I have encountered 'sed's that don't take a file name as argument.

This patch is just code churn for which you give no convincing reason in the commit message why it is good.

-- Hannes