# [PATCH] grep: add configuration variables for --heading 6 messages from 2020-05-26 to 2020-06-08. Participants: Simon Ser, Eric Sunshine, Junio C Hamano, Phillip Wood. Thread: https://gitlist.dev/t/53548 ## Simon Ser, 2020-05-26 08:37 Subject: [PATCH] grep: add configuration variables for --heading Message-ID: <4WK2xPaKLXPbLE0jjY0rBR9M_BVt69rzJKBN17M0ily4VPYuMNC_a2kXsIKWR8y6lc7j58WpBmaIqdbJ1whnjonrfEWz7YE-WpdQqCu_lN0=@emersion.fr> URL: https://gitlist.dev/e/4WK2xPaKLXPbLE0jjY0rBR9M_BVt69rzJKBN17M0ily4VPYuMNC_a2kXsIKWR8y6lc7j58WpBmaIqdbJ1whnjonrfEWz7YE-WpdQqCu_lN0%3D%40emersion.fr ``` 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 --- Documentation/git-grep.txt | 3 +++ grep.c | 5 +++++ 2 files changed, 8 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. + grep.patternType:: Set the default matching behavior. Using a value of 'basic', 'extended', 'fixed', or 'perl' will enable the `--basic-regexp`, `--extended-regexp`, diff --git a/grep.c b/grep.c index 13232a904aca..4549bc722650 100644 --- a/grep.c +++ b/grep.c @@ -133,6 +133,10 @@ int grep_config(const char *var, const char *value, void *cb) opt->columnnum = git_config_bool(var, value); return 0; } + if (!strcmp(var, "grep.heading")) { + opt->heading = git_config_bool(var, value); + return 0; + } if (!strcmp(var, "grep.fullname")) { opt->relative = !git_config_bool(var, value); @@ -199,6 +203,7 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix opt->pattern_type_option = def->pattern_type_option; opt->linenum = def->linenum; opt->columnnum = def->columnnum; + opt->heading = def->heading; opt->max_depth = def->max_depth; opt->pathname = def->pathname; opt->relative = def->relative; -- 2.26.2 ``` ## Eric Sunshine, 2020-05-26 13:40 Subject: Re: [PATCH] grep: add configuration variables for --heading Message-ID: URL: https://gitlist.dev/e/CAPig%2BcTkwUyyji19XyTBRvC%3DRjLNK_OipLJ-%2B6GTf8PWRfC8WA%40mail.gmail.com In-Reply-To: <4WK2xPaKLXPbLE0jjY0rBR9M_BVt69rzJKBN17M0ily4VPYuMNC_a2kXsIKWR8y6lc7j58WpBmaIqdbJ1whnjonrfEWz7YE-WpdQqCu_lN0=@emersion.fr> ``` On Tue, May 26, 2020 at 4:37 AM Simon Ser wrote: > 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 > --- > Documentation/git-grep.txt | 3 +++ > grep.c | 5 +++++ > 2 files changed, 8 insertions(+) To round this out, adding a test -- probably in t7810-grep.sh -- would be a good idea. ``` ## Simon Ser, 2020-05-27 15:45 Subject: [PATCH v2] grep: add configuration variables for --heading Message-ID: URL: https://gitlist.dev/e/C5PlGbi78_QpceNrgFm_-Wt6BooqbntaL8OdH7XjxjXCtyIfsisyeOQMslLOwNY68Dg7iUxGIvzLPAvUEWyMIC0GdpTgeWwgwD5sWlsyrSM%3D%40emersion.fr In-Reply-To: <4WK2xPaKLXPbLE0jjY0rBR9M_BVt69rzJKBN17M0ily4VPYuMNC_a2kXsIKWR8y6lc7j58WpBmaIqdbJ1whnjonrfEWz7YE-WpdQqCu_lN0=@emersion.fr> ``` 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 --- 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. + grep.patternType:: Set the default matching behavior. Using a value of 'basic', 'extended', 'fixed', or 'perl' will enable the `--basic-regexp`, `--extended-regexp`, diff --git a/grep.c b/grep.c index 13232a904aca..4549bc722650 100644 --- a/grep.c +++ b/grep.c @@ -133,6 +133,10 @@ int grep_config(const char *var, const char *value, void *cb) opt->columnnum = git_config_bool(var, value); return 0; } + if (!strcmp(var, "grep.heading")) { + opt->heading = git_config_bool(var, value); + return 0; + } if (!strcmp(var, "grep.fullname")) { opt->relative = !git_config_bool(var, value); @@ -199,6 +203,7 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix opt->pattern_type_option = def->pattern_type_option; opt->linenum = def->linenum; opt->columnnum = def->columnnum; + opt->heading = def->heading; opt->max_depth = def->max_depth; opt->pathname = def->pathname; opt->relative = def->relative; 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 +' + cat >expected <hello.c 4:int main(int argc, const char **argv) -- 2.26.2 ``` ## Junio C Hamano, 2020-05-27 16:45 Subject: Re: [PATCH v2] grep: add configuration variables for --heading Message-ID: URL: https://gitlist.dev/e/xmqqeer5mina.fsf%40gitster.c.googlers.com In-Reply-To: ``` Simon Ser 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 > --- > > 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 < hello.c > 4:int main(int argc, const char **argv) ``` ## Simon Ser, 2020-06-08 10:22 Subject: Re: [PATCH v2] grep: add configuration variables for --heading Message-ID: <2lg0C_0k4SnWqhB0oW3wGE6eixtc_ZT5GgZJFzH8XC6g5dJXmSGGnrV9uZVT4QIOG9Sjc7yuLOR_v5kwGIV1hnMSojXinNG16G3tOOMg-Fc=@emersion.fr> URL: https://gitlist.dev/e/2lg0C_0k4SnWqhB0oW3wGE6eixtc_ZT5GgZJFzH8XC6g5dJXmSGGnrV9uZVT4QIOG9Sjc7yuLOR_v5kwGIV1hnMSojXinNG16G3tOOMg-Fc%3D%40emersion.fr In-Reply-To: ``` On Wednesday, May 27, 2020 6:45 PM, Junio C Hamano wrote: > 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. Is see where you're getting at, but this is missing for a handful of options, like grep.lineNumber and grep.column. I'd rather not create an inconsistency here. > 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). Note, these new tests are only required if --no-heading is added to the patch. ``` ## Phillip Wood, 2020-06-08 14:02 Subject: Re: [PATCH v2] grep: add configuration variables for --heading Message-ID: <1c61583e-5af4-2c11-0147-213b1c484fbd@gmail.com> URL: https://gitlist.dev/e/1c61583e-5af4-2c11-0147-213b1c484fbd%40gmail.com In-Reply-To: <2lg0C_0k4SnWqhB0oW3wGE6eixtc_ZT5GgZJFzH8XC6g5dJXmSGGnrV9uZVT4QIOG9Sjc7yuLOR_v5kwGIV1hnMSojXinNG16G3tOOMg-Fc=@emersion.fr> ``` Hi Simon On 08/06/2020 11:22, Simon Ser wrote: > On Wednesday, May 27, 2020 6:45 PM, Junio C Hamano wrote: > >> 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. > > Is see where you're getting at, but this is missing for a handful of > options, like grep.lineNumber and grep.column. I'd rather not create an > inconsistency here. Those options do exist - try git -c grep.column=true grep --no-column ... and you will see that the column number is not printed. This is because OPT_BOOL() without the PARSE_OPT_NONEG flag creates as --no- option. If these existing options are not documented or tested perhaps you would be able to add a second patch to fix that. >> 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). > > Note, these new tests are only required if --no-heading is added to the > patch. That is not correct in the case of `git grep -c grep.heading=nonsense grep ...`. Anyway --no-heading does exist. Best Wishes Phillip ```