From: Martin von Zweigbergk <martinvonz@gmail.com>
Date: Tue, 23 Apr 2013 16:51:28 GMT
Subject: Re: [PATCH 5/7] rebase -i: return control to the caller, for housekeeping
Message-ID: <CANiSa6hmh1d7ECBCAm+AKFV4KXh7eoHfk7pL0RjaqZj4XbKhpw@mail.gmail.com>
In-Reply-To: <1366725724-1016-6-git-send-email-artagnon@gmail.com>

On Tue, Apr 23, 2013 at 7:02 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index cc3a9a7..9514e31 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -597,7 +597,7 @@ do_next () {
>                 fi
>                 ;;
>         esac
> -       test -s "$todo" && return
> +       test -s "$todo" && return 1

Unlike the other cases below, this seems to be replacing success by
failure. What does it mean in practice that $todo is empty?

>         comment_for_reflog finish &&
>         newhead=$(git rev-parse HEAD) &&
> @@ -623,17 +623,15 @@ do_next () {
>                 "$GIT_DIR"/hooks/post-rewrite rebase < "$rewritten_list"
>                 true # we don't care if this hook failed
>         fi &&
> -       rm -rf "$state_dir" &&
> -       git gc --auto &&
>         warn "Successfully rebased and updated $head_name."
>
> -       exit
> +       return 0

So after this patch, the "warning" will coming before gc is run. It's
a change, but it seems fine. gc usually only prints a few line, right?

>  }
>
>  do_rest () {
>         while :
>         do
> -               do_next
> +               do_next && break
>         done
>  }

Normally one would break if unsuccessful. What would fail if this was
replaced by "do_next || break" and the above ".. && return 1" was "..
&& return". I assume that was your first attempt, but why did it not
work?