Re: [PATCH] rev-list: restore the NUL commit separator in --header mode

From: Jacob Keller <jacob.keller@gmail.com>
Date: Oct 19, 2016, 22:15 UTC
Message-ID: <CA+P7+xogHOCbPV+rx7yrur85m=HX5ms9kGQYvTpQ7n2i7Hzuvw@mail.gmail.com>
In-Reply-To: <20161019210448.aupphybw5qar6mqe@hurricane>

Hi,

On Wed, Oct 19, 2016 at 2:04 PM, Dennis Kaarsemaker <dennis@kaarsemaker.net> wrote:

Show 5 quoted lines

> Commit 660e113 (graph: add support for --line-prefix on all graph-aware
> output) changed the way commits were shown. Unfortunately this dropped
> the NUL between commits in --header mode. Restore the NUL and add a test
> for this feature.
>

Oops! Thanks for the bug fix.

Show 18 quoted lines

> Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
> ---
>  builtin/rev-list.c       | 4 ++++
>  t/t6000-rev-list-misc.sh | 7 +++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 8479f6e..cfa6a7d 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -157,6 +157,10 @@ static void show_commit(struct commit *commit, void *data)
>                         if (revs->commit_format == CMIT_FMT_ONELINE)
>                                 putchar('\n');
>                 }
> +               if (revs->commit_format == CMIT_FMT_RAW) {
> +                       putchar(info->hdr_termination);
> +               }
> +

This seems right to me. My one concern is that we make sure we restore it for every case (in case it needs to be there for other formats?) I'm not entirely sure about whether other non-raw modes need this or not?

Thanks, Jake