Re: [PATCH v2] grep: add configuration variables for --heading

From: Junio C Hamano <gitster@pobox.com>
Date: May 27, 2020, 16:45 UTC
Message-ID: <xmqqeer5mina.fsf@gitster.c.googlers.com>
In-Reply-To: <C5PlGbi78_QpceNrgFm_-Wt6BooqbntaL8OdH7XjxjXCtyIfsisyeOQMslLOwNY68Dg7iUxGIvzLPAvUEWyMIC0GdpTgeWwgwD5sWlsyrSM=@emersion.fr>

Simon Ser <contact@emersion.fr> writes:

Show 24 quoted lines

> There are already configuration variables for -n and --column. Add one
> for --heading, allowing users to customize the default behaviour.
>
> Signed-off-by: Simon Ser <contact@emersion.fr>
> ---
>
> v2: add a test
>
>  Documentation/git-grep.txt | 3 +++
>  grep.c                     | 5 +++++
>  t/t7810-grep.sh            | 5 +++++
>  3 files changed, 13 insertions(+)
>
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index a7f9bc99eaf1..ed4f05d885a2 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -47,6 +47,9 @@ grep.lineNumber::
>  grep.column::
>  	If set to true, enable the `--column` option by default.
>  
> +grep.heading::
> +	If set to true, enable the `--heading` option by default.
> +

OK.

Naturally it follows that a command-line option

	$ git config grep.heading yes
	$ git grep --no-heading -e pattern

is a way to countermand the configured default per invocation basis.

Show 13 quoted lines

> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 991d5bd9c03f..1acc6fe89c51 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -1556,6 +1556,11 @@ test_expect_success 'grep --heading' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'grep with grep.heading=true' '
> +	git -c grep.heading=true grep -e char -e lo_w hello.c hello_world >actual &&
> +	test_cmp expected actual
> +'
> +

When writing new tests, we often get too excited and stop at showing off how well the shiny new feature works, but we should make sure that we test the "negative" case, too, i.e. that the "feature" can be disabled when the user does not want to trigger it, and that the "feature" notices incorrect invocations and fails appropriately.

E.g.

	git -c grep.heading=yes grep --no-heading ...

should not leave the opt->heading true, and

	git -c grep.heading=nonsense grep ...

should fail, saying "grep.heading must be a bool" (or something along that line).

Thanks.

>  cat >expected <<EOF
>  <BOLD;GREEN>hello.c<RESET>
>  4:int main(int argc, const <BLACK;BYELLOW>char<RESET> **argv)