On Sun, Jul 11, 2021 at 11:55 PM brian m. carlson
<sandals@xxxxxxxxxxxxxxxxxxxx> 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 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@xxxxxxxxxxxxxxxxxxxx>
> ---
> 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.
Thanks! This version mostly looks good to me!
> 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
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>]".)
> test_format () {
> + local args=
Here...
> + while true
> + do
> + case "$1" in
> + --*)
> + args="$args $1"
...here...
> + 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 &&
...and here "opts" might be better than "args".
> + test_cmp expect.$1 output.$1
> + "
> +}
> +
> +# usage: test_pretty [argument...] name format_name [failure] <expected_output
Here...
> +test_pretty () {
> + local args=
...and in this function too.
> + 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
test_pretty() accepts options like --no-commit-header, but it's only
used without any option. So I wonder if you forgot to add a few tests
with some options.
> +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
