{"thread":{"id":"31152","subject":"[PATCH/RFC] grep: add a perlRegexp configuration option","startedAt":"2012-07-31T16:57:34Z","lastAt":"2012-07-31T22:56:32Z","messageCount":7,"participants":["J Smith","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"196239","messageId":"1343753854-66765-1-git-send-email-dark.panda@gmail.com","threadId":"31152","inReplyTo":null,"subject":"[PATCH/RFC] grep: add a perlRegexp configuration option","fromName":"J Smith","fromEmail":"dark.panda@gmail.com","sentAt":"2012-07-31T16:57:34Z","receivedAt":"2012-07-31T16:57:34Z","isPatch":true,"sender":{"key":"dark.panda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/84783?v=4"},"body":"Enables the -P flag for perl regexps by default. When both the\nperlRegexp and extendedRegexp options are enabled, the last enabled\noption wins.\n---\n Documentation/config.txt   |  6 ++++++\n Documentation/git-grep.txt |  6 ++++++\n builtin/grep.c             | 17 +++++++++++++++--\n t/t7810-grep.sh            | 34 ++++++++++++++++++++++++++++++++++\n 4 files changed, 61 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex a95e5a4..ff3019b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1213,6 +1213,12 @@ grep.lineNumber::\n grep.extendedRegexp::\n \tIf set to true, enable '--extended-regexp' option by default.\n\n+grep.perlRegexp::\n+\tIf set to true, enable '--perl-regexp' option by default.\n+\n+When both the 'grep.extendedRegexp' and 'grep.perlRegexp' options\n+are used, the last enabled option wins.\n+\n gpg.program::\n \tUse this custom program instead of \"gpg\" found on $PATH when\n \tmaking or verifying a PGP signature. The program must support the\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 3bec036..8816968 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -45,6 +45,12 @@ grep.lineNumber::\n grep.extendedRegexp::\n \tIf set to true, enable '--extended-regexp' option by default.\n\n+grep.perlRegexp::\n+\tIf set to true, enable '--perl-regexp' option by default.\n+\n+When both the 'grep.extendedRegexp' and 'grep.perlRegexp' options\n+are used, the last enabled option wins.\n+\n\n OPTIONS\n -------\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 29adb0a..b4475e6 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -268,11 +268,24 @@ static int grep_config(const char *var, const char *value, void *cb)\n \tif (userdiff_config(var, value) < 0)\n \t\treturn -1;\n\n+\tif (!strcmp(var, \"grep.perlregexp\")) {\n+\t\tif (git_config_bool(var, value)) {\n+\t\t\topt->fixed = 0;\n+\t\t\topt->pcre = 1;\n+\t\t} else {\n+\t\t\topt->pcre = 0;\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"grep.extendedregexp\")) {\n-\t\tif (git_config_bool(var, value))\n+\t\tif (git_config_bool(var, value)) {\n \t\t\topt->regflags |= REG_EXTENDED;\n-\t\telse\n+\t\t\topt->pcre = 0;\n+\t\t\topt->fixed = 0;\n+\t\t} else {\n \t\t\topt->regflags &= ~REG_EXTENDED;\n+\t\t}\n \t\treturn 0;\n \t}\n\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 24e9b19..5479dc9 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -729,6 +729,40 @@ test_expect_success LIBPCRE 'grep -P pattern' '\n \ttest_cmp expected actual\n '\n\n+test_expect_success LIBPCRE 'grep pattern with grep.perlRegexp=true' '\n+\tgit \\\n+\t\t-c grep.perlregexp=true \\\n+\t\tgrep \"\\p{Ps}.*?\\p{Pe}\" hello.c >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success LIBPCRE 'grep pattern with grep.perlRegexp=true and then grep.extendedRegexp=true' '\n+\ttest_must_fail git \\\n+\t\t-c grep.perlregexp=true \\\n+\t\t-c grep.extendedregexp=true \\\n+\t\tgrep \"\\p{Ps}.*?\\p{Pe}\" hello.c\n+'\n+\n+test_expect_success LIBPCRE 'grep pattern with grep.extendedRegexp=true and then grep.perlRegexp=true' '\n+\tgit \\\n+\t\t-c grep.extendedregexp=true \\\n+\t\t-c grep.perlregexp=true \\\n+\t\tgrep \"\\p{Ps}.*?\\p{Pe}\" hello.c >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success LIBPCRE 'grep -E pattern with grep.perlRegexp=true' '\n+\ttest_must_fail git \\\n+\t\t-c grep.perlregexp=true \\\n+\t\tgrep -E \"\\p{Ps}.*?\\p{Pe}\" hello.c\n+'\n+\n+test_expect_success LIBPCRE 'grep -G pattern with grep.perlRegexp=true' '\n+\ttest_must_fail git \\\n+\t\t-c grep.perlregexp=true \\\n+\t\tgrep -G \"\\p{Ps}.*?\\p{Pe}\" hello.c\n+'\n+\n test_expect_success 'grep pattern with grep.extendedRegexp=true' '\n \t>empty &&\n \ttest_must_fail git -c grep.extendedregexp=true \\\n--\n1.7.11.3\n"},{"id":"196243","messageId":"7vehnrkdrq.fsf@alter.siamese.dyndns.org","threadId":"31152","inReplyTo":"1343753854-66765-1-git-send-email-dark.panda@gmail.com","subject":"Re: [PATCH/RFC] grep: add a perlRegexp configuration option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-31T18:04:57Z","receivedAt":"2012-07-31T18:04:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"J Smith <dark.panda@gmail.com> writes:\n\n> Enables the -P flag for perl regexps by default. When both the\n> perlRegexp and extendedRegexp options are enabled, the last enabled\n> option wins.\n\nTurning \"grep.extendedregexp\" from boolean to an extended boolean to\nallow \"grep.extendedregexp = perl\" might be a better alternative.\nThat way, the user wouldn't have to worry about 7 variants of\ngrep.fooRegexp variables twenty years down the road, even though the\nset of possible values given to \"grep.extendedregexp\" may have grown\nover time by then.\n\n> ---\n>  Documentation/config.txt   |  6 ++++++\n>  Documentation/git-grep.txt |  6 ++++++\n>  builtin/grep.c             | 17 +++++++++++++++--\n>  t/t7810-grep.sh            | 34 ++++++++++++++++++++++++++++++++++\n>  4 files changed, 61 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index a95e5a4..ff3019b 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1213,6 +1213,12 @@ grep.lineNumber::\n>  grep.extendedRegexp::\n>  \tIf set to true, enable '--extended-regexp' option by default.\n>\n> +grep.perlRegexp::\n> +\tIf set to true, enable '--perl-regexp' option by default.\n> +\n> +When both the 'grep.extendedRegexp' and 'grep.perlRegexp' options\n> +are used, the last enabled option wins.\n> +\n>  gpg.program::\n>  \tUse this custom program instead of \"gpg\" found on $PATH when\n>  \tmaking or verifying a PGP signature. The program must support the\n> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\n> index 3bec036..8816968 100644\n> --- a/Documentation/git-grep.txt\n> +++ b/Documentation/git-grep.txt\n> @@ -45,6 +45,12 @@ grep.lineNumber::\n>  grep.extendedRegexp::\n>  \tIf set to true, enable '--extended-regexp' option by default.\n>\n> +grep.perlRegexp::\n> +\tIf set to true, enable '--perl-regexp' option by default.\n> +\n> +When both the 'grep.extendedRegexp' and 'grep.perlRegexp' options\n> +are used, the last enabled option wins.\n> +\n>\n>  OPTIONS\n>  -------\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 29adb0a..b4475e6 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -268,11 +268,24 @@ static int grep_config(const char *var, const char *value, void *cb)\n>  \tif (userdiff_config(var, value) < 0)\n>  \t\treturn -1;\n>\n> +\tif (!strcmp(var, \"grep.perlregexp\")) {\n> +\t\tif (git_config_bool(var, value)) {\n> +\t\t\topt->fixed = 0;\n> +\t\t\topt->pcre = 1;\n> +\t\t} else {\n> +\t\t\topt->pcre = 0;\n> +\t\t}\n> +\t\treturn 0;\n> +\t}\n> +\n>  \tif (!strcmp(var, \"grep.extendedregexp\")) {\n> -\t\tif (git_config_bool(var, value))\n> +\t\tif (git_config_bool(var, value)) {\n>  \t\t\topt->regflags |= REG_EXTENDED;\n> -\t\telse\n> +\t\t\topt->pcre = 0;\n> +\t\t\topt->fixed = 0;\n> +\t\t} else {\n>  \t\t\topt->regflags &= ~REG_EXTENDED;\n> +\t\t}\n>  \t\treturn 0;\n>  \t}\n>\n> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\n> index 24e9b19..5479dc9 100755\n> --- a/t/t7810-grep.sh\n> +++ b/t/t7810-grep.sh\n> @@ -729,6 +729,40 @@ test_expect_success LIBPCRE 'grep -P pattern' '\n>  \ttest_cmp expected actual\n>  '\n>\n> +test_expect_success LIBPCRE 'grep pattern with grep.perlRegexp=true' '\n> +\tgit \\\n> +\t\t-c grep.perlregexp=true \\\n> +\t\tgrep \"\\p{Ps}.*?\\p{Pe}\" hello.c >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_expect_success LIBPCRE 'grep pattern with grep.perlRegexp=true and then grep.extendedRegexp=true' '\n> +\ttest_must_fail git \\\n> +\t\t-c grep.perlregexp=true \\\n> +\t\t-c grep.extendedregexp=true \\\n> +\t\tgrep \"\\p{Ps}.*?\\p{Pe}\" hello.c\n> +'\n> +\n> +test_expect_success LIBPCRE 'grep pattern with grep.extendedRegexp=true and then grep.perlRegexp=true' '\n> +\tgit \\\n> +\t\t-c grep.extendedregexp=true \\\n> +\t\t-c grep.perlregexp=true \\\n> +\t\tgrep \"\\p{Ps}.*?\\p{Pe}\" hello.c >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_expect_success LIBPCRE 'grep -E pattern with grep.perlRegexp=true' '\n> +\ttest_must_fail git \\\n> +\t\t-c grep.perlregexp=true \\\n> +\t\tgrep -E \"\\p{Ps}.*?\\p{Pe}\" hello.c\n> +'\n> +\n> +test_expect_success LIBPCRE 'grep -G pattern with grep.perlRegexp=true' '\n> +\ttest_must_fail git \\\n> +\t\t-c grep.perlregexp=true \\\n> +\t\tgrep -G \"\\p{Ps}.*?\\p{Pe}\" hello.c\n> +'\n> +\n>  test_expect_success 'grep pattern with grep.extendedRegexp=true' '\n>  \t>empty &&\n>  \ttest_must_fail git -c grep.extendedregexp=true \\\n> --\n> 1.7.11.3\n"},{"id":"196261","messageId":"CADFUPgfHQCzdnXfby5b+z-pRkt5o6MAVEf_1waWER3iVtM1TZA@mail.gmail.com","threadId":"31152","inReplyTo":"7vehnrkdrq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] grep: add a perlRegexp configuration option","fromName":"J Smith","fromEmail":"dark.panda@gmail.com","sentAt":"2012-07-31T20:20:28Z","receivedAt":"2012-07-31T20:20:28Z","isPatch":true,"sender":{"key":"dark.panda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/84783?v=4"},"body":"On Tue, Jul 31, 2012 at 2:04 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Turning \"grep.extendedregexp\" from boolean to an extended boolean to\n> allow \"grep.extendedregexp = perl\" might be a better alternative.\n> That way, the user wouldn't have to worry about 7 variants of\n> grep.fooRegexp variables twenty years down the road, even though the\n> set of possible values given to \"grep.extendedregexp\" may have grown\n> over time by then.\n\nYeah, that sounds good. I've re-written the patch to accommodate the\nchange allowing for the current boolean settings of true/false as well\nas \"perl\". For the sake of completeness (verbosity? pedantry?) I also\nincluded a setting for \"extended\" which is equivalent to true.\n\nWith this sort of change, would a more generic \"grep.regexpMode\",\n\"grep.patternType\" or something similar perhaps be more descriptive,\nwith \"grep.extendedRegexp\" being aliased for backwards compatibility\npurposes? I could also add that functionality if desired.\n"},{"id":"196264","messageId":"7vboivishe.fsf@alter.siamese.dyndns.org","threadId":"31152","inReplyTo":"CADFUPgfHQCzdnXfby5b+z-pRkt5o6MAVEf_1waWER3iVtM1TZA@mail.gmail.com","subject":"Re: [PATCH/RFC] grep: add a perlRegexp configuration option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-31T20:30:05Z","receivedAt":"2012-07-31T20:30:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"J Smith <dark.panda@gmail.com> writes:\n\n> ... For the sake of completeness (verbosity? pedantry?) I also\n> included a setting for \"extended\" which is equivalent to true.\n\nGood thinking.\n\n> With this sort of change, would a more generic \"grep.regexpMode\",\n> \"grep.patternType\" or something similar perhaps be more descriptive,\n> with \"grep.extendedRegexp\" being aliased for backwards compatibility\n> purposes? I could also add that functionality if desired.\n\nA variable called \"extendedRegexp\" already reads quite naturally if\nit can have value to say what kind of extendedness is desired, at\nleast to me.  So I do not care too deeply either way.\n"},{"id":"196265","messageId":"CADFUPgfrG75FkSNV6JS22DAciD3CVkNEjKncNJOPkfRQSf41Pg@mail.gmail.com","threadId":"31152","inReplyTo":"7vboivishe.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] grep: add a perlRegexp configuration option","fromName":"J Smith","fromEmail":"dark.panda@gmail.com","sentAt":"2012-07-31T20:35:44Z","receivedAt":"2012-07-31T20:35:44Z","isPatch":true,"sender":{"key":"dark.panda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/84783?v=4"},"body":"On Tue, Jul 31, 2012 at 4:30 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> J Smith <dark.panda@gmail.com> writes:\n>\n>> ... For the sake of completeness (verbosity? pedantry?) I also\n>> included a setting for \"extended\" which is equivalent to true.\n>\n> Good thinking.\n>\n>> With this sort of change, would a more generic \"grep.regexpMode\",\n>> \"grep.patternType\" or something similar perhaps be more descriptive,\n>> with \"grep.extendedRegexp\" being aliased for backwards compatibility\n>> purposes? I could also add that functionality if desired.\n>\n> A variable called \"extendedRegexp\" already reads quite naturally if\n> it can have value to say what kind of extendedness is desired, at\n> least to me.  So I do not care too deeply either way.\n\nOn the flip side, it might be useful to some to have the option to set\nthe value to \"fixed\" for the \"--fixed-strings\" argument, in which case\nthe option becomes less a type of extended regexp and more of a simple\nsearch string. Were that to be the case, I think \"grep.patternType\"\nwould feel the most precise.\n\nI think for completeness at the very least I should work in the\n\"fixed\" value as an valid value, option naming aside.\n"},{"id":"196267","messageId":"7v3947iqup.fsf@alter.siamese.dyndns.org","threadId":"31152","inReplyTo":"CADFUPgfrG75FkSNV6JS22DAciD3CVkNEjKncNJOPkfRQSf41Pg@mail.gmail.com","subject":"Re: [PATCH/RFC] grep: add a perlRegexp configuration option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-31T21:05:18Z","receivedAt":"2012-07-31T21:05:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"J Smith <dark.panda@gmail.com> writes:\n\n> On Tue, Jul 31, 2012 at 4:30 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> J Smith <dark.panda@gmail.com> writes:\n>>\n>>> ... For the sake of completeness (verbosity? pedantry?) I also\n>>> included a setting for \"extended\" which is equivalent to true.\n>>\n>> Good thinking.\n>>\n>>> With this sort of change, would a more generic \"grep.regexpMode\",\n>>> \"grep.patternType\" or something similar perhaps be more descriptive,\n>>> with \"grep.extendedRegexp\" being aliased for backwards compatibility\n>>> purposes? I could also add that functionality if desired.\n>>\n>> A variable called \"extendedRegexp\" already reads quite naturally if\n>> it can have value to say what kind of extendedness is desired, at\n>> least to me.  So I do not care too deeply either way.\n>\n> On the flip side, it might be useful to some to have the option to set\n> the value to \"fixed\" for the \"--fixed-strings\" argument, in which case\n> the option becomes less a type of extended regexp and more of a simple\n> search string. Were that to be the case, I think \"grep.patternType\"\n> would feel the most precise.\n>\n> I think for completeness at the very least I should work in the\n> \"fixed\" value as an valid value, option naming aside.\n\nOk, then grep.patternType it is.\n\nThanks.\n"},{"id":"196272","messageId":"CADFUPgejLSOcQrmcCGuDt8PYdHFun=UYqCshroZpdcV65inikA@mail.gmail.com","threadId":"31152","inReplyTo":"7v3947iqup.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] grep: add a perlRegexp configuration option","fromName":"J Smith","fromEmail":"dark.panda@gmail.com","sentAt":"2012-07-31T22:56:32Z","receivedAt":"2012-07-31T22:56:32Z","isPatch":true,"sender":{"key":"dark.panda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/84783?v=4"},"body":"On Tue, Jul 31, 2012 at 5:05 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Ok, then grep.patternType it is.\n>\n> Thanks.\n\nCool, patches should be on their way. I added options for \"basic\",\n\"extended\", \"fixed\" and \"perl\" for completeness along with the name\nchange with a BC alias patch separately for perusal.\n\nCheers.\n"}]}