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

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

Simon Ser <contact@emersion.fr> writes:

> 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.

> 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)