Re: [PATCH] rev-list: add option for --pretty without header

[Junio C Hamano <gitster@pobox.com>]

Christian Couder <christian.couder@gmail.com> writes:

> On Wed, Jul 7, 2021 at 12:47 AM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
>>
>> In general, we encourage users to use plumbing commands, like git
>> rev-list, over porcelain commands, like git log, when scripting.
>> However, git rev-list has one glaring problem that prevents it from
>> being used in certain cases: when --pretty is used, it always prints out
>> a line containing "commit" and the object ID.
>
> You say always, but it looks like it doesn't do it when
> --pretty=oneline is used.

I've understood that he meant "when --pretty=format is used",
though, as it is the only format whose intent was to allow
generation of output stream that does not have to be preprocessed
with "grep -v '^[0-9a-f]{40}'" etc.

> It looks like the tests only check what happens in case
> --pretty=format:'...' is used, but I wonder what the code does if a
> builtin format is used.

Good point.

I also think the handling of --abbrev-commit may need to be
rethought with this change.  See here:

    diff --git a/builtin/rev-list.c b/builtin/rev-list.c
    index 7677b1af5a..f571cc9598 100644
    --- a/builtin/rev-list.c
    +++ b/builtin/rev-list.c
    @@ -132,7 +132,7 @@ static void show_commit(struct commit *commit, void *data)
            if (revs->abbrev_commit && revs->abbrev)
                    fputs(find_unique_abbrev(&commit->object.oid, revs->abbrev),
                          stdout);
    -	else
    +	else if (revs->include_header)
                    fputs(oid_to_hex(&commit->object.oid), stdout);
            if (revs->print_parents) {
                    struct commit_list *parents = commit->parents;

The original says that if --abbrev-commit is set and --abbrev is set
to non-zero, we'd show unique abbreviation and if not, specifically,
even when --abbrev-commit is set but --abbrev is set to 0, we didn't
do the find_unique_abbrev() of full hexdigits but left the output to
the else clause.  This was because the original code KNOWS that the
else clause unconditionally emits the full commit object name.

That assumption, which made the original code's handling of
"--abbrev-commit --abbrev=0" correct, no longer holds with the
updated code.  What happens is with --no-commit-header, if we give
"--abbrev-commit --abbrev=4", we still see the unique abbreviation
that is not shorter than 4 hexdigits (i.e. "--no-commit-header" is
ignored), but if we say "--abbrev-commit --abbrev=0", we do not see
any commit header.

My hunch is that this "if / else", which determines if the commit
header should give shortened or full length, should be skipped when
the .include_header is false.