patchrev-list: add option for --pretty without header

>
> 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.

>  --header::
>         Print the contents of the commit in raw-format; each record is
>         separated with a NUL character.
> +
> +--no-commit-header::
> +       Suppress the header line containing "commit" and the object ID printed before
> +       the specified format.

> +--commit-header::
> +       Overrides a previous `--no-commit-header`.
>  endif::git-rev-list[]
>
>  --parents::

> diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
> index 35a2f62392..e68bf9f21c 100755
> --- a/t/t6006-rev-list-format.sh
> +++ b/t/t6006-rev-list-format.sh
> @@ -54,9 +54,15 @@ test_expect_success 'setup' '
>
>  # usage: test_format name format_string [failure] <expected_output
>  test_format () {
> +       local header_arg=
> +       if test "$1" = "--no-commit-header"
> +       then
> +               header_arg="--no-commit-header"
> +               shift
> +       fi
>         cat >expect.$1
>         test_expect_${3:-success} "format $1" "
> -               git rev-list --pretty=format:'$2' main >output.$1 &&
> +               git rev-list $header_arg --pretty=format:'$2' main >output.$1 &&
>                 test_cmp expect.$1 output.$1
>         "
>  }

> 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.

> 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.

>
> 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 with a custom format,
> it always prints out a line containing "commit" and the object ID.  This
> makes it unsuitable for many scripting needs, and forces users to use
> git log instead.
>
> While we can't change this behavior for backwards compatibility, we can
> add an option to suppress this behavior, so let's do so, and call it
> "--no-commit-header".  Additionally, add the corresponding positive
> option to switch it back on.
>
> Note that this option doesn't affect the built-in formats, only custom
> formats.  This is exactly the same behavior as users already have from
> git log and is what most users will be used to.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> Changes from v1:
> * Ensure this applies only to custom formats.
> * Fix the issue with --abbrev-commit mentioned by Junio.
> * Add tests for additional cases, including built-in formats.
> * Update documentation to reflect this data.

> diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
> index 35a2f62392..41d0ca00b1 100755
> --- a/t/t6006-rev-list-format.sh
> +++ b/t/t6006-rev-list-format.sh
> @@ -41,22 +41,59 @@ test_expect_success 'setup' '
>         echo "$added_iso88591" | git commit -F - &&
>         head1=$(git rev-parse --verify HEAD) &&
>         head1_short=$(git rev-parse --verify --short $head1) &&
> +       head1_short4=$(git rev-parse --verify --short=4 $head1) &&
>         tree1=$(git rev-parse --verify HEAD:) &&
>         tree1_short=$(git rev-parse --verify --short $tree1) &&
>         echo "$changed" > foo &&
>         echo "$changed_iso88591" | git commit -a -F - &&
>         head2=$(git rev-parse --verify HEAD) &&
>         head2_short=$(git rev-parse --verify --short $head2) &&
> +       head2_short4=$(git rev-parse --verify --short=4 $head2) &&
>         tree2=$(git rev-parse --verify HEAD:) &&
>         tree2_short=$(git rev-parse --verify --short $tree2) &&
>         git config --unset i18n.commitEncoding
>  '
>
> -# usage: test_format name format_string [failure] <expected_output
> +# usage: test_format [argument...] name format_string [failure] <expected_output

> +       while true
> +       do
> +               case "$1" in
> +               --*)
> +                       args="$args $1"

> +                       shift;;
> +               *)
> +                       break;;
> +               esac
> +       done
>         cat >expect.$1
>         test_expect_${3:-success} "format $1" "
> -               git rev-list --pretty=format:'$2' main >output.$1 &&
> +               git rev-list $args --pretty=format:'$2' main >output.$1 &&

> +               test_cmp expect.$1 output.$1
> +       "
> +}
> +
> +# usage: test_pretty [argument...] name format_name [failure] <expected_output

> +       while true
> +       do
> +               case "$1" in
> +               --*)
> +                       args="$args $1"
> +                       shift;;
> +               *)
> +                       break;;
> +               esac
> +       done
> +       cat >expect.$1
> +       test_expect_${3:-success} "pretty $1 (without --no-commit-header)" "
> +               git rev-list $args --pretty='$2' main >output.$1 &&
> +               test_cmp expect.$1 output.$1
> +       "
> +       test_expect_${3:-success} "pretty $1 (with --no-commit-header)" "
> +               git rev-list $args --no-commit-header --pretty='$2' main >output.$1 &&
>                 test_cmp expect.$1 output.$1
>         "
>  }

> +test_pretty oneline oneline <<EOF
> +$head2 $changed
> +$head1 $added
> +EOF
> +
> +test_pretty short short <<EOF

> +commit $head2
> +Author: $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>
> +
> +    $changed
> +
> +commit $head1
> +Author: $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>
> +
> +    $added
> +
> +EOF
> +
>  test_expect_success 'basic colors' '
>         cat >expect <<-EOF &&
>         commit $head2

> Usually we use "option" instead of "argument" for the flags starting
> with "-" or "--" before the required parameter. For example:
> 
> $ git rev-list -h
> usage: git rev-list [OPTION] <commit-id>... [ -- paths... ]
> ...
> 
> (Yeah, I agree that [OPTION] is not very consistent with what other
> commands show, which is usually "[<options>]".)

> 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 with a custom format,
> it always prints out a line containing "commit" and the object ID.  This
> makes it unsuitable for many scripting needs, and forces users to use
> git log instead.
> 
> While we can't change this behavior for backwards compatibility, we can
> add an option to suppress this behavior, so let's do so, and call it
> "--no-commit-header".  Additionally, add the corresponding positive
> option to switch it back on.
> 
> Note that this option doesn't affect the built-in formats, only custom
> formats.  This is exactly the same behavior as users already have from
> git log and is what most users will be used to.

> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 5bf2a85f69..23388f36c3 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -1064,6 +1064,14 @@ ifdef::git-rev-list[]
>  --header::
>  	Print the contents of the commit in raw-format; each record is
>  	separated with a NUL character.
> +
> +--no-commit-header::
> +	Suppress the header line containing "commit" and the object ID printed before
> +	the specified format.  This has no effect on the built-in formats; only custom
> +	formats are affected.

> It wasn't immediately obvious to me what "custom formats" meant here. I
> don't think we use that term elsewhere, nor do we seem to have a
> succinct phrase for the concept. Maybe something like:
> 
>   only custom formats (i.e., `--pretty=format:`) are affected.
> 
> helps without making it too clunky?