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

[Christian Couder <christian.couder@gmail.com>]

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.

By the way this makes me think that the doc could be improved, because
it describes the format this way:

      •   online

              <hash> <title line>

      •   short

              commit <hash>
              Author: <author>

              <title line>

      •   reference

              <abbrev hash> (<title line>, <short author date>)

      •   email

              From <hash> <date>
              From: <author>
              Date: <author date>
              Subject: [PATCH] <title line>

              <full commit message>

while when --pretty=reference or --pretty=email is used the 'commit
<hash>' line is still printed despite not being shown in the doc.

> This makes it unsuitable
> for many scripting needs, and forces users to use git log instead.

I agree that it's annoying when using --pretty=format:'...'

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

It's not clear if this new option will remove the 'commit <hash>' line
when a builtin format like --pretty=short, --pretty=medium,
--pretty=reference or --pretty=email is used.

> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> This has come up a lot on the list and I'm sure most of the regulars
> have run into it before.

Yeah, I did run into it before. Thanks for doing something about it!

> It came up at $DAYJOB and I said I intended to
> send a patch, so here it is.
>
> I chose --no-commit-header because --header is already taken.

Yeah, that looks like the most obvious option name.

>  --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 might want to tell what happens with builtin formats.

> +--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
>         "
>  }

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.