Re: [Outreachy PATCH] annotate: pass the arguments to blame unchanged

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

Tanga Joris <tangajoris@gmail.com> writes:

> cmd_annotate() puts "-c" in front of the user's arguments before
> calling cmd_blame(), so "--git-completion-helper" is no longer the
> lone argument that parse-options looks for. It falls through to the
> diff option parser, which answers it. So "git annotate --<TAB>"
> offers the options of "git diff" instead of annotate's own. There is
> no --porcelain, while "git blame --<TAB>" offers it.

Makes sense.

>  builtin/annotate.c            | 22 +---------------------
>  t/t8003-blame-corner-cases.sh | 10 ++++++++++
>  2 files changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/builtin/annotate.c b/builtin/annotate.c
> index 7f754f2309..c1f258993d 100644
> --- a/builtin/annotate.c
> +++ b/builtin/annotate.c
> @@ -6,31 +6,11 @@
>  
>  #include "git-compat-util.h"
>  #include "builtin.h"
> -#include "strvec.h"
>  
>  int cmd_annotate(int argc,
>  		 const char **argv,
>  		 const char *prefix,
>  		 struct repository *repo)
>  {
> -	struct strvec args = STRVEC_INIT;
> -	const char **args_copy;
> -	int ret;
> -
> -	strvec_pushl(&args, "annotate", "-c", NULL);
> -	for (int i = 1; i < argc; i++)
> -		strvec_push(&args, argv[i]);
> -
> -	/*
> -	 * `cmd_blame()` ends up modifying the array, which causes memory leaks
> -	 * if we didn't copy the array here.
> -	 */
> -	CALLOC_ARRAY(args_copy, args.nr + 1);
> -	COPY_ARRAY(args_copy, args.v, args.nr);
> -
> -	ret = cmd_blame(args.nr, args_copy, prefix, repo);
> -
> -	strvec_clear(&args);
> -	free(args_copy);
> -	return ret;
> +	return cmd_blame(argc, argv, prefix, repo);
>  }

Nice simplification.

> diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
> index 30e7960ace..6824f33332 100755
> --- a/t/t8003-blame-corner-cases.sh
> +++ b/t/t8003-blame-corner-cases.sh
> @@ -303,4 +303,14 @@ test_expect_success 'blame does not coalesce non-adjacent result lines' '
>  	test_cmp expect actual
>  '
>  
> +for opt in --git-completion-helper --git-completion-helper-all
> +do
> +	test_expect_success "annotate $opt offers the same options as blame" '

Making "annotate" trigger completion-helper is a great goal.  "git
help annotate" does show the same set of options as "blame", so it
is in line with what the commands actually support.

Looking good.  Thanks.

> +		git blame $opt >expect &&
> +		git annotate $opt >actual &&
> +		test_grep -e "--porcelain" actual &&
> +		test_cmp expect actual
> +	'
> +done
> +
>  test_done
>
> base-commit: 0ffa3e35c7c82ab8d0de226149fb03b3f7674d05