Re: [PATCH v2 06/26] test: completion: add run_func() helper
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 11, 2020, 07:27 UTC
- Message-ID
- <xmqq1rh05p6f.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20201110212136.870769-7-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> Pretty straightforward: runs functions.
Hmph, sorry but this is not straight-forward at least to me. Yes, the helper runs whatever is given on the command line, but then it does "print_comp", too. And the proposed log message is not entirely clear on the most important thing: why?
What is this "helper" meant to help? Reduce repetition?
Show 16 quoted lines
> +run_func ()
> +{
> + local -a COMPREPLY &&
> + "$@" && print_comp
> +}
> +
> # Test high-level completion
> # Arguments are:
> # 1: typed text so far (cur)
> @@ -452,8 +458,7 @@ test_expect_success '__gitcomp_direct - puts everything into COMPREPLY as-is' '
> EOF
> (
> cur=should_be_ignored &&
> - __gitcomp_direct "$(cat expected)" &&
> - print_comp
> + run_func __gitcomp_direct "$(cat expected)"This is an no-op rewrite, as we used to do the gitcomp-direct followed by print-comp, which is exactly what the helper does. So the helper does reduce repetition, which by itself would be a good thing but is there other benefit we are trying to seek (there does not have to be any)?
> test_expect_success '__gitcomp - doesnt fail because of invalid variable name' ' > - __gitcomp "$invalid_variable_name" > + run_func __gitcomp "$invalid_variable_name"
This one changes the behaviour in that it starts running print_comp which we didn't run. It may be a good thing and improvement, but then we'd better advertise it in the proposed log message.
Show 9 quoted lines
> ' > > read -r -d "" refs <<-\EOF > @@ -586,7 +591,7 @@ test_expect_success '__gitcomp_nl - no suffix' ' > ' > > test_expect_success '__gitcomp_nl - doesnt fail because of invalid variable name' ' > - __gitcomp_nl "$invalid_variable_name" > + run_func __gitcomp_nl "$invalid_variable_name"
Likewise.
> '
Thanks.