From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 08 Jun 2015 21:29:13 GMT
Subject: Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
Message-ID: <xmqqioax99ee.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1433797202-97425-2-git-send-email-rappazzo@gmail.com>

Michael Rappazzo <rappazzo@gmail.com> writes:

> A config option 'rebase.instructionFormat' can override the
> default 'oneline' format of the rebase instruction list.
>
> Since the list is parsed using the left, right or boundary mark plus
> the sha1, they are prepended to the instruction format.
>
> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
> ---
>  git-rebase--interactive.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index dc3133f..b92375e 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -977,7 +977,9 @@ else
>  	revisions=$onto...$orig_head
>  	shortrevisions=$shorthead
>  fi
> -git rev-list $merges_option --pretty=oneline --reverse --left-right --topo-order \
> +format=$(git config --get rebase.instructionFormat)
> +# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
> +git rev-list $merges_option --format="%m%H ${format-%s}" --reverse --left-right --topo-order \
>  	$revisions ${restrict_revision+^$restrict_revision} | \
>  	sed -n "s/^>//p" |
>  while read -r sha1 rest

Looks OK, but

 - Needs a patch to Documentation/config.txt and possibly also
   Documentation/git-rebase.txt

 - Needs tests somewhere in t34?? series.

Also I think "git rev-list" line has got way too long.  As it is
already using backslash-continuation, do not hesitate to fold it
further, e.g. 

        git rev-list $merges_option --format="%m%H ${format-%s}" \
                --reverse --left-right --topo-order \
                $revisions ${restrict_revision+^$restrict_revision} | \
                sed -n "s/^>//p" |
	while ...

Thanks.