{"thread":{"id":"53548","subject":"[PATCH] grep: add configuration variables for --heading","startedAt":"2020-05-26T08:37:36Z","lastAt":"2020-06-08T14:02:54Z","messageCount":6,"participants":["Simon Ser","Eric Sunshine","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"398573","messageId":"4WK2xPaKLXPbLE0jjY0rBR9M_BVt69rzJKBN17M0ily4VPYuMNC_a2kXsIKWR8y6lc7j58WpBmaIqdbJ1whnjonrfEWz7YE-WpdQqCu_lN0=@emersion.fr","threadId":"53548","inReplyTo":null,"subject":"[PATCH] grep: add configuration variables for --heading","fromName":"Simon Ser","fromEmail":"contact@emersion.fr","sentAt":"2020-05-26T08:37:23Z","receivedAt":"2020-05-26T08:37:36Z","isPatch":true,"sender":{"key":"contact@emersion.fr","avatar":"https://avatars.githubusercontent.com/u/506932?v=4"},"body":"There are already configuration variables for -n and --column. Add one\nfor --heading, allowing users to customize the default behaviour.\n\nSigned-off-by: Simon Ser <contact@emersion.fr>\n---\n Documentation/git-grep.txt | 3 +++\n grep.c                     | 5 +++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex a7f9bc99eaf1..ed4f05d885a2 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -47,6 +47,9 @@ grep.lineNumber::\n grep.column::\n \tIf set to true, enable the `--column` option by default.\n \n+grep.heading::\n+\tIf set to true, enable the `--heading` option by default.\n+\n grep.patternType::\n \tSet the default matching behavior. Using a value of 'basic', 'extended',\n \t'fixed', or 'perl' will enable the `--basic-regexp`, `--extended-regexp`,\ndiff --git a/grep.c b/grep.c\nindex 13232a904aca..4549bc722650 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -133,6 +133,10 @@ int grep_config(const char *var, const char *value, void *cb)\n \t\topt->columnnum = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"grep.heading\")) {\n+\t\topt->heading = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \tif (!strcmp(var, \"grep.fullname\")) {\n \t\topt->relative = !git_config_bool(var, value);\n@@ -199,6 +203,7 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix\n \topt->pattern_type_option = def->pattern_type_option;\n \topt->linenum = def->linenum;\n \topt->columnnum = def->columnnum;\n+\topt->heading = def->heading;\n \topt->max_depth = def->max_depth;\n \topt->pathname = def->pathname;\n \topt->relative = def->relative;\n-- \n2.26.2\n\n\n"},{"id":"398577","messageId":"CAPig+cTkwUyyji19XyTBRvC=RjLNK_OipLJ-+6GTf8PWRfC8WA@mail.gmail.com","threadId":"53548","inReplyTo":"4WK2xPaKLXPbLE0jjY0rBR9M_BVt69rzJKBN17M0ily4VPYuMNC_a2kXsIKWR8y6lc7j58WpBmaIqdbJ1whnjonrfEWz7YE-WpdQqCu_lN0=@emersion.fr","subject":"Re: [PATCH] grep: add configuration variables for --heading","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-05-26T13:40:29Z","receivedAt":"2020-05-26T13:40:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 26, 2020 at 4:37 AM Simon Ser <contact@emersion.fr> wrote:\n> There are already configuration variables for -n and --column. Add one\n> for --heading, allowing users to customize the default behaviour.\n>\n> Signed-off-by: Simon Ser <contact@emersion.fr>\n> ---\n>  Documentation/git-grep.txt | 3 +++\n>  grep.c                     | 5 +++++\n>  2 files changed, 8 insertions(+)\n\nTo round this out, adding a test -- probably in t7810-grep.sh -- would\nbe a good idea.\n"},{"id":"398639","messageId":"C5PlGbi78_QpceNrgFm_-Wt6BooqbntaL8OdH7XjxjXCtyIfsisyeOQMslLOwNY68Dg7iUxGIvzLPAvUEWyMIC0GdpTgeWwgwD5sWlsyrSM=@emersion.fr","threadId":"53548","inReplyTo":"4WK2xPaKLXPbLE0jjY0rBR9M_BVt69rzJKBN17M0ily4VPYuMNC_a2kXsIKWR8y6lc7j58WpBmaIqdbJ1whnjonrfEWz7YE-WpdQqCu_lN0=@emersion.fr","subject":"[PATCH v2] grep: add configuration variables for --heading","fromName":"Simon Ser","fromEmail":"contact@emersion.fr","sentAt":"2020-05-27T15:45:46Z","receivedAt":"2020-05-27T15:46:44Z","isPatch":true,"sender":{"key":"contact@emersion.fr","avatar":"https://avatars.githubusercontent.com/u/506932?v=4"},"body":"There are already configuration variables for -n and --column. Add one\nfor --heading, allowing users to customize the default behaviour.\n\nSigned-off-by: Simon Ser <contact@emersion.fr>\n---\n\nv2: add a test\n\n Documentation/git-grep.txt | 3 +++\n grep.c                     | 5 +++++\n t/t7810-grep.sh            | 5 +++++\n 3 files changed, 13 insertions(+)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex a7f9bc99eaf1..ed4f05d885a2 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -47,6 +47,9 @@ grep.lineNumber::\n grep.column::\n \tIf set to true, enable the `--column` option by default.\n \n+grep.heading::\n+\tIf set to true, enable the `--heading` option by default.\n+\n grep.patternType::\n \tSet the default matching behavior. Using a value of 'basic', 'extended',\n \t'fixed', or 'perl' will enable the `--basic-regexp`, `--extended-regexp`,\ndiff --git a/grep.c b/grep.c\nindex 13232a904aca..4549bc722650 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -133,6 +133,10 @@ int grep_config(const char *var, const char *value, void *cb)\n \t\topt->columnnum = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"grep.heading\")) {\n+\t\topt->heading = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \n \tif (!strcmp(var, \"grep.fullname\")) {\n \t\topt->relative = !git_config_bool(var, value);\n@@ -199,6 +203,7 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix\n \topt->pattern_type_option = def->pattern_type_option;\n \topt->linenum = def->linenum;\n \topt->columnnum = def->columnnum;\n+\topt->heading = def->heading;\n \topt->max_depth = def->max_depth;\n \topt->pathname = def->pathname;\n \topt->relative = def->relative;\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 991d5bd9c03f..1acc6fe89c51 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -1556,6 +1556,11 @@ test_expect_success 'grep --heading' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'grep with grep.heading=true' '\n+\tgit -c grep.heading=true grep -e char -e lo_w hello.c hello_world >actual &&\n+\ttest_cmp expected actual\n+'\n+\n cat >expected <<EOF\n <BOLD;GREEN>hello.c<RESET>\n 4:int main(int argc, const <BLACK;BYELLOW>char<RESET> **argv)\n-- \n2.26.2\n\n\n"},{"id":"398644","messageId":"xmqqeer5mina.fsf@gitster.c.googlers.com","threadId":"53548","inReplyTo":"C5PlGbi78_QpceNrgFm_-Wt6BooqbntaL8OdH7XjxjXCtyIfsisyeOQMslLOwNY68Dg7iUxGIvzLPAvUEWyMIC0GdpTgeWwgwD5sWlsyrSM=@emersion.fr","subject":"Re: [PATCH v2] grep: add configuration variables for --heading","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-27T16:45:13Z","receivedAt":"2020-05-27T16:45:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Simon Ser <contact@emersion.fr> writes:\n\n> There are already configuration variables for -n and --column. Add one\n> for --heading, allowing users to customize the default behaviour.\n>\n> Signed-off-by: Simon Ser <contact@emersion.fr>\n> ---\n>\n> v2: add a test\n>\n>  Documentation/git-grep.txt | 3 +++\n>  grep.c                     | 5 +++++\n>  t/t7810-grep.sh            | 5 +++++\n>  3 files changed, 13 insertions(+)\n>\n> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\n> index a7f9bc99eaf1..ed4f05d885a2 100644\n> --- a/Documentation/git-grep.txt\n> +++ b/Documentation/git-grep.txt\n> @@ -47,6 +47,9 @@ grep.lineNumber::\n>  grep.column::\n>  \tIf set to true, enable the `--column` option by default.\n>  \n> +grep.heading::\n> +\tIf set to true, enable the `--heading` option by default.\n> +\n\nOK.\n\nNaturally it follows that a command-line option\n\n\t$ git config grep.heading yes\n\t$ git grep --no-heading -e pattern\n\nis a way to countermand the configured default per invocation\nbasis.\n\n> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\n> index 991d5bd9c03f..1acc6fe89c51 100755\n> --- a/t/t7810-grep.sh\n> +++ b/t/t7810-grep.sh\n> @@ -1556,6 +1556,11 @@ test_expect_success 'grep --heading' '\n>  \ttest_cmp expected actual\n>  '\n>  \n> +test_expect_success 'grep with grep.heading=true' '\n> +\tgit -c grep.heading=true grep -e char -e lo_w hello.c hello_world >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n\nWhen writing new tests, we often get too excited and stop at showing\noff how well the shiny new feature works, but we should make sure\nthat we test the \"negative\" case, too, i.e. that the \"feature\" can\nbe disabled when the user does not want to trigger it, and that the\n\"feature\" notices incorrect invocations and fails appropriately.\n\nE.g.\n\n\tgit -c grep.heading=yes grep --no-heading ...\n\nshould not leave the opt->heading true, and\n\n\tgit -c grep.heading=nonsense grep ...\n\nshould fail, saying \"grep.heading must be a bool\" (or something\nalong that line).\n\nThanks.\n\n>  cat >expected <<EOF\n>  <BOLD;GREEN>hello.c<RESET>\n>  4:int main(int argc, const <BLACK;BYELLOW>char<RESET> **argv)\n"},{"id":"399289","messageId":"2lg0C_0k4SnWqhB0oW3wGE6eixtc_ZT5GgZJFzH8XC6g5dJXmSGGnrV9uZVT4QIOG9Sjc7yuLOR_v5kwGIV1hnMSojXinNG16G3tOOMg-Fc=@emersion.fr","threadId":"53548","inReplyTo":"xmqqeer5mina.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] grep: add configuration variables for --heading","fromName":"Simon Ser","fromEmail":"contact@emersion.fr","sentAt":"2020-06-08T10:22:36Z","receivedAt":"2020-06-08T10:23:03Z","isPatch":true,"sender":{"key":"contact@emersion.fr","avatar":"https://avatars.githubusercontent.com/u/506932?v=4"},"body":"On Wednesday, May 27, 2020 6:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n\n> Naturally it follows that a command-line option\n>\n> $ git config grep.heading yes\n> $ git grep --no-heading -e pattern\n>\n> is a way to countermand the configured default per invocation\n> basis.\n\nIs see where you're getting at, but this is missing for a handful of\noptions, like grep.lineNumber and grep.column. I'd rather not create an\ninconsistency here.\n\n> When writing new tests, we often get too excited and stop at showing\n> off how well the shiny new feature works, but we should make sure\n> that we test the \"negative\" case, too, i.e. that the \"feature\" can\n> be disabled when the user does not want to trigger it, and that the\n> \"feature\" notices incorrect invocations and fails appropriately.\n>\n> E.g.\n>\n> git -c grep.heading=yes grep --no-heading ...\n>\n> should not leave the opt->heading true, and\n>\n> git -c grep.heading=nonsense grep ...\n>\n> should fail, saying \"grep.heading must be a bool\" (or something\n> along that line).\n\nNote, these new tests are only required if --no-heading is added to the\npatch.\n"},{"id":"399295","messageId":"1c61583e-5af4-2c11-0147-213b1c484fbd@gmail.com","threadId":"53548","inReplyTo":"2lg0C_0k4SnWqhB0oW3wGE6eixtc_ZT5GgZJFzH8XC6g5dJXmSGGnrV9uZVT4QIOG9Sjc7yuLOR_v5kwGIV1hnMSojXinNG16G3tOOMg-Fc=@emersion.fr","subject":"Re: [PATCH v2] grep: add configuration variables for --heading","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-06-08T14:02:48Z","receivedAt":"2020-06-08T14:02:54Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Simon\n\nOn 08/06/2020 11:22, Simon Ser wrote:\n> On Wednesday, May 27, 2020 6:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> \n>> Naturally it follows that a command-line option\n>>\n>> $ git config grep.heading yes\n>> $ git grep --no-heading -e pattern\n>>\n>> is a way to countermand the configured default per invocation\n>> basis.\n> \n> Is see where you're getting at, but this is missing for a handful of\n> options, like grep.lineNumber and grep.column. I'd rather not create an\n> inconsistency here.\n\nThose options do exist - try\n   git -c grep.column=true grep --no-column ...\nand you will see that the column number is not printed. This is because \nOPT_BOOL() without the PARSE_OPT_NONEG flag creates as --no- option.\n\nIf these existing options are not documented or tested perhaps you would \nbe able to add a second patch to fix that.\n\n>> When writing new tests, we often get too excited and stop at showing\n>> off how well the shiny new feature works, but we should make sure\n>> that we test the \"negative\" case, too, i.e. that the \"feature\" can\n>> be disabled when the user does not want to trigger it, and that the\n>> \"feature\" notices incorrect invocations and fails appropriately.\n>>\n>> E.g.\n>>\n>> git -c grep.heading=yes grep --no-heading ...\n>>\n>> should not leave the opt->heading true, and\n>>\n>> git -c grep.heading=nonsense grep ...\n>>\n>> should fail, saying \"grep.heading must be a bool\" (or something\n>> along that line).\n> \n> Note, these new tests are only required if --no-heading is added to the\n> patch.\n\nThat is not correct in the case of `git grep -c grep.heading=nonsense \ngrep ...`. Anyway --no-heading does exist.\n\nBest Wishes\n\nPhillip\n"}]}