SZEDER Gábor <szeder@xxxxxxxxxx> writes:
> 'git describe --contains' doesn't default to HEAD when no commit is
> given, and it doesn't produce any output, not even an error:
>
> ~/src/git ((v2.5.0))$ ./git describe --contains
> ~/src/git ((v2.5.0))$ ./git describe --contains HEAD
> v2.5.0^0
Good spotting. I think defaulting to HEAD is a good move.
> diff --git a/builtin/describe.c b/builtin/describe.c
> index ce36032b1f..0e31ac5cb9 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -443,10 +443,13 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
> if (pattern)
> argv_array_pushf(&args, "--refs=refs/tags/%s", pattern);
> }
> - while (*argv) {
> - argv_array_push(&args, *argv);
> - argv++;
> - }
> + if (argc)
"What would this code do to 'describe --all --contains'?" was my
knee-jerk reaction, but the options are all parsed by this code and
nothing is delegated to name-rev, so 'if (!argc)' here is truly the
lack of any revisions to be described, which is good.
> + while (*argv) {
> + argv_array_push(&args, *argv);
> + argv++;
> + }
> + else
> + argv_array_push(&args, "HEAD");
By the way, I usually prefer a fatter 'else' clause when everything
else is equal, i.e.
if (!argc)
argv_array_push(&args, "HEAD"); /* default to HEAD */
else {
while (*argv) {
...
}
}
because it is easy to miss tiny else-clause while reading code, but
it is harder to miss tiny then-clause. In this case, however, the
while loop can be replaced with argv_array_pushv() these days, so
perhaps
if (!argc)
argv_array_push(&args, "HEAD"); /* default to HEAD ... */
else
argv_array_pushv(&args, argv); /* or relay what we got */
or something?
> return cmd_name_rev(args.argc, args.argv, prefix);
> }
>
> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> index 57d50156bb..bf52a0a1cc 100755
> --- a/t/t6120-describe.sh
> +++ b/t/t6120-describe.sh
> @@ -115,6 +115,14 @@ check_describe e-3-* --first-parent --tags
>
> check_describe $(git rev-parse --short HEAD) --exact-match --always HEAD
>
> +test_expect_success 'describe --contains defaults to HEAD without commit-ish' '
> + echo "A^0" >expect &&
> + git checkout A &&
> + test_when_finished "git checkout -" &&
> + git describe --contains >actual &&
> + test_cmp expect actual
> +'
> : >err.expect
> check_describe A --all A^0
> test_expect_success 'no warning was displayed for A' '
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html