From: David Aguilar <davvid@gmail.com>
Date: Tue, 10 Jun 2014 18:43:47 GMT
Subject: Re: [PATCH 09/20] git-rebase--interactive.sh: avoid "test <cond> -a/-o <cond>"
Message-ID: <20140610184346.GC72751@gmail.com>
In-Reply-To: <1402066563-28519-10-git-send-email-gitter.spiros@gmail.com>

On Fri, Jun 06, 2014 at 07:55:52AM -0700, Elia Pinto wrote:
> The construct is error-prone; "test" being built-in in most modern
> shells, the reason to avoid "test <cond> && test <cond>" spawning
> one extra process by using a single "test <cond> -a <cond>" no
> longer exists.
> 
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
>  git-rebase--interactive.sh |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 6ec9d3c..797571f 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -1013,7 +1013,7 @@ then
>  	git rev-list $revisions |
>  	while read rev
>  	do
> -		if test -f "$rewritten"/$rev -a "$(sane_grep "$rev" "$state_dir"/not-cherry-picks)" = ""
> +		if test -f "$rewritten"/$rev && test "$(sane_grep "$rev" "$state_dir"/not-cherry-picks)" = ""
>  		then

This line is getting pretty long.
I wonder whether it should be wrapped at the && to keep it shorter?

Also, it looks like the last half of the expression can be simplified to,

	test -z "$(sane_grep "$rev" "$state_dir"/not-cherry-picks)"

rather than comparing against "".
-- 
David