{"thread":{"id":"41155","subject":"[PATCH 0/3] Implicitly use --no-index if git grep is used outside of repo","startedAt":"2016-01-10T14:19:54Z","lastAt":"2016-01-12T15:50:05Z","messageCount":32,"participants":["Thomas Gummerer","Duy Nguyen","Eric Sunshine","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"275611","messageId":"1452435597-12099-1-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":null,"subject":"[PATCH 0/3] Implicitly use --no-index if git grep is used outside of repo","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-10T14:19:54Z","receivedAt":"2016-01-10T14:19:54Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Often when looking for something in a downloaded source archive I find\nmyself using git grep --no-index after forgetting to use --no-index\nthe first time and git unhelpfully dies.\n\nThis patch series makes git grep implicitly run git grep --no-index\nwhen run outside of a repository, analogous to how git diff works when\nrun outside of a repository.\n\nThomas Gummerer (3):\n t7810: correct --no-index test\n builtin/grep: rename use_index to no_index\n builtin/grep: allow implicit --no-index\n\n builtin/grep.c | 40 ++++++++++++++++++++++++++++------------\n t/t7810-grep.sh | 45 ++++++++++++++++++++++++++++++++++++++++-----\n 2 files changed, 68 insertions(+), 17 deletions(-)\n\n-- \n2.6.3.387.g749a69c.dirty\n"},{"id":"275614","messageId":"1452435597-12099-2-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452435597-12099-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH 1/3] t7810: correct --no-index test","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-10T14:19:55Z","receivedAt":"2016-01-10T14:19:55Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"GIT_CEILING_DIRECTORIES doesn't prevent chdir up into another directory\nwhile looking for a repository directory if it is equal to the current\ndirectory. Because of this, the test which claims to test the git grep\n--no-index command outside of a repository actually tests it inside of a\nrepository. The test_must_fail assertions still pass because the git\ngrep only looks at untracked files and therefore no file matches, but\nnot because it's run outside of a repository as it was originally\nintended.\n\nSet the GIT_CEILING_DIRECTORIES environment variable to the parent\ndirectory of the directory in which the git grep command is executed, to\nmake sure it is actually run outside of a git repository.\n\nSigned-off-by: Thomas Gummerer \n---\n\n t/t7810-grep.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 028ffe4..ab94716 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -791,7 +791,7 @@ test_expect_success 'outside of git repository' '\n \t} >non/expect.full &&\n \techo file2:world >non/expect.sub &&\n \t(\n-\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non/git\" &&\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n \t\ttest_must_fail git grep o &&\n@@ -805,7 +805,7 @@ test_expect_success 'outside of git repository' '\n \n \techo \".*o*\" >non/git/.gitignore &&\n \t(\n-\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non/git\" &&\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n \t\ttest_must_fail git grep o &&\n-- \n2.6.3.387.g749a69c.dirty\n"},{"id":"275613","messageId":"1452435597-12099-3-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452435597-12099-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH 2/3] builtin/grep: rename use_index to no_index","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-10T14:19:56Z","receivedAt":"2016-01-10T14:19:56Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Switch the OPT_NEGBIT argument to a OPT_BIT argument, and rename the\ncorresponding use_index variable to a no_index variable. This will make\nthe following step easier to follow. No functional changes intended.\n\nSigned-off-by: Thomas Gummerer \n---\n builtin/grep.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 4229cae..3a27bd5 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -625,14 +625,14 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tstruct string_list path_list = STRING_LIST_INIT_NODUP;\n \tint i;\n \tint dummy;\n-\tint use_index = 1;\n+\tint no_index = 0;\n \tint pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"cached\", &cached,\n \t\t\tN_(\"search in index instead of in the work tree\")),\n-\t\tOPT_NEGBIT(0, \"no-index\", &use_index,\n-\t\t\t N_(\"find in contents not managed by git\"), 1),\n+\t\tOPT_BIT(0, \"no-index\", &no_index,\n+\t\t\tN_(\"find in contents not managed by git\"), 1),\n \t\tOPT_BOOL(0, \"untracked\", &untracked,\n \t\t\tN_(\"search in both tracked and untracked files\")),\n \t\tOPT_SET_INT(0, \"exclude-standard\", &opt_exclude,\n@@ -755,7 +755,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n \n-\tif (use_index && !startup_info->have_repository)\n+\tif (!no_index && !startup_info->have_repository)\n \t\t/* die the same way as if we did it at the beginning */\n \t\tsetup_git_directory();\n \n@@ -873,11 +873,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tif (!show_in_pager && !opt.status_only)\n \t\tsetup_pager();\n \n-\tif (!use_index && (untracked || cached))\n+\tif (no_index && (untracked || cached))\n \t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n \n-\tif (!use_index || untracked) {\n-\t\tint use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude;\n+\tif (no_index || untracked) {\n+\t\tint use_exclude = (opt_exclude < 0) ? !no_index : !!opt_exclude;\n \t\tif (list.nr)\n \t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n \t\thit = grep_directory(&opt, &pathspec, use_exclude);\n-- \n2.6.3.387.g749a69c.dirty\n"},{"id":"275612","messageId":"1452435597-12099-4-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452435597-12099-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-10T14:19:57Z","receivedAt":"2016-01-10T14:19:57Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Currently when git grep is used outside of a git repository without the\n--no-index option git simply dies. For convenience, implicitly make git\ngrep behave like git grep --no-index when it is called outside of a git\nrepository.\n\nSigned-off-by: Thomas Gummerer \n---\n builtin/grep.c | 32 ++++++++++++++++++++++++--------\n t/t7810-grep.sh | 41 ++++++++++++++++++++++++++++++++++++++---\n 2 files changed, 62 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 3a27bd5..a886af1 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -19,6 +19,9 @@\n #include \"dir.h\"\n #include \"pathspec.h\"\n \n+#define GREP_NO_INDEX_EXPLICIT 1\n+#define GREP_NO_INDEX_IMPLICIT 2\n+\n static char const * const grep_usage[] = {\n \tN_(\"git grep [] [-e] [...] [[--] ...]\"),\n \tNULL\n@@ -632,7 +635,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"cached\", &cached,\n \t\t\tN_(\"search in index instead of in the work tree\")),\n \t\tOPT_BIT(0, \"no-index\", &no_index,\n-\t\t\tN_(\"find in contents not managed by git\"), 1),\n+\t\t\tN_(\"find in contents not managed by git\"),\n+\t\t\tGREP_NO_INDEX_EXPLICIT),\n \t\tOPT_BOOL(0, \"untracked\", &untracked,\n \t\t\tN_(\"search in both tracked and untracked files\")),\n \t\tOPT_SET_INT(0, \"exclude-standard\", &opt_exclude,\n@@ -755,9 +759,13 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n \n-\tif (!no_index && !startup_info->have_repository)\n-\t\t/* die the same way as if we did it at the beginning */\n-\t\tsetup_git_directory();\n+\tif (!no_index && !startup_info->have_repository) {\n+\t\tint nongit = 0;\n+\n+\t\tsetup_git_directory_gently(&nongit);\n+\t\tif (nongit)\n+\t\t\tno_index = GREP_NO_INDEX_IMPLICIT;\n+\t}\n \n \t/*\n \t * skip a -- separator; we know it cannot be\n@@ -873,13 +881,21 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tif (!show_in_pager && !opt.status_only)\n \t\tsetup_pager();\n \n-\tif (no_index && (untracked || cached))\n-\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n+\tif (untracked || cached) {\n+\t\tif (no_index == GREP_NO_INDEX_EXPLICIT)\n+\t\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n+\t\telse if (no_index == GREP_NO_INDEX_IMPLICIT)\n+\t\t\tdie(_(\"--cached or --untracked cannot be used outside a git repository.\"));\n+\t}\n \n \tif (no_index || untracked) {\n \t\tint use_exclude = (opt_exclude < 0) ? !no_index : !!opt_exclude;\n-\t\tif (list.nr)\n-\t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n+\t\tif (list.nr) {\n+\t\t\tif (no_index == GREP_NO_INDEX_IMPLICIT)\n+\t\t\t\tdie(_(\"cannot use revs outside of a git repository.\"));\n+\t\t\telse\n+\t\t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n+\t\t}\n \t\thit = grep_directory(&opt, &pathspec, use_exclude);\n \t} else if (0 <= opt_exclude) {\n \t\tdie(_(\"--[no-]exclude-standard cannot be used for tracked contents.\"));\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex ab94716..4ba955d 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -794,11 +794,9 @@ test_expect_success 'outside of git repository' '\n \t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n-\t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index o >../actual.full &&\n \t\ttest_cmp ../expect.full ../actual.full\n \t\tcd sub &&\n-\t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index o >../../actual.sub &&\n \t\ttest_cmp ../../expect.sub ../../actual.sub\n \t) &&\n@@ -808,7 +806,6 @@ test_expect_success 'outside of git repository' '\n \t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n-\t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index --exclude-standard o >../actual.full &&\n \t\ttest_cmp ../expect.full ../actual.full &&\n \n@@ -821,6 +818,44 @@ test_expect_success 'outside of git repository' '\n \t)\n '\n \n+test_expect_success 'outside of git repository without --no-index' '\n+\trm -fr non &&\n+\tmkdir -p non/git/sub &&\n+\techo hello >non/git/file1 &&\n+\techo world >non/git/sub/file2 &&\n+\t{\n+\t\techo file1:hello &&\n+\t\techo sub/file2:world\n+\t} >non/expect.full &&\n+\techo file2:world >non/expect.sub &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\tgit grep o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full\n+\t\tcd sub &&\n+\t\tgit grep o >../../actual.sub &&\n+\t\ttest_cmp ../../expect.sub ../../actual.sub\n+\t) &&\n+\n+\techo \".*o*\" >non/git/.gitignore &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\tgit grep --exclude-standard o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\n+\t\t{\n+\t\t\techo \".gitignore:.*o*\"\n+\t\t\tcat ../expect.full\n+\t\t} >../expect.with.ignored &&\n+\t\tgit grep --no-exclude o >../actual.full &&\n+\t\ttest_cmp ../expect.with.ignored ../actual.full\n+\t)\n+'\n+\n test_expect_success 'inside git repository but with --no-index' '\n \trm -fr is &&\n \tmkdir -p is/git/sub &&\n-- \n2.6.3.387.g749a69c.dirty\n"},{"id":"275621","messageId":"CACsJy8Bs3z0Gk3CjhyZGfOLA7R3pZQz7K5gk4BTytvYkZeyBtQ@mail.gmail.com","threadId":"41155","inReplyTo":"1452435597-12099-4-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-01-11T00:50:40Z","receivedAt":"2016-01-11T00:50:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jan 10, 2016 at 9:19 PM, Thomas Gummerer wrote:\n> Currently when git grep is used outside of a git repository without the\n> --no-index option git simply dies. For convenience, implicitly make git\n> grep behave like git grep --no-index when it is called outside of a git\n> repository.\n\nShould we have a line about this behavior in git-grep.txt, maybe the\ndescription section? I wonder if anybody wants the old behavior (e.g.\nnon-zero exit code when running outside a repo). If there is such a\ncase (*), we may need an option to revert it back (--no-no-index seems\nridiculous, maybe --use-index). The safest way though, is introduce a\nnew option like --use-index= then you can make\nan grep alias with --use-index=optional.\n\n(*) I've been hitting really weird real-world use cases so I'm a bit paranoid..\n-- \nDuy\n"},{"id":"275622","messageId":"CAPig+cRXy=CBZBTC7fU5wHA3d9Acqh_WYD54DmY1sHj7rob9Pw@mail.gmail.com","threadId":"41155","inReplyTo":"1452435597-12099-4-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-01-11T01:54:31Z","receivedAt":"2016-01-11T01:54:31Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jan 10, 2016 at 9:19 AM, Thomas Gummerer wrote:\n> Currently when git grep is used outside of a git repository without the\n> --no-index option git simply dies. For convenience, implicitly make git\n> grep behave like git grep --no-index when it is called outside of a git\n> repository.\n>\n> Signed-off-by: Thomas Gummerer \n> ---\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> @@ -19,6 +19,9 @@\n> +#define GREP_NO_INDEX_EXPLICIT 1\n> +#define GREP_NO_INDEX_IMPLICIT 2\n> @@ -873,13 +881,21 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> if (!show_in_pager && !opt.status_only)\n> setup_pager();\n>\n> - if (no_index && (untracked || cached))\n> - die(_(\"--cached or --untracked cannot be used with --no-index.\"));\n> + if (untracked || cached) {\n> + if (no_index == GREP_NO_INDEX_EXPLICIT)\n> + die(_(\"--cached or --untracked cannot be used with --no-index.\"));\n> + else if (no_index == GREP_NO_INDEX_IMPLICIT)\n\nJust below here when checking --untracked, you use a simple 'else'\nrather than 'else if' to handle the other case of explicit vs\nimplicit. Why the inconsistency? Also, the ordering of 'if/else' arms\nis opposite.\n\n> + die(_(\"--cached or --untracked cannot be used outside a git repository.\"));\n> + }\n>\n> if (no_index || untracked) {\n> int use_exclude = (opt_exclude < 0) ? !no_index : !!opt_exclude;\n> - if (list.nr)\n> - die(_(\"--no-index or --untracked cannot be used with revs.\"));\n> + if (list.nr) {\n> + if (no_index == GREP_NO_INDEX_IMPLICIT)\n> + die(_(\"cannot use revs outside of a git repository.\"));\n> + else\n> + die(_(\"--no-index or --untracked cannot be used with revs.\"));\n> + }\n> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\n> @@ -821,6 +818,44 @@ test_expect_success 'outside of git repository' '\n> +test_expect_success 'outside of git repository without --no-index' '\n> + rm -fr non &&\n> + mkdir -p non/git/sub &&\n> + echo hello >non/git/file1 &&\n> + echo world >non/git/sub/file2 &&\n> + {\n> + echo file1:hello &&\n> + echo sub/file2:world\n> + } >non/expect.full &&\n\nIsn't the above just a complicated way of saying:\n\n cat <<-\\EOF >non/expect.full &&\n file:hello\n sub/file2:world\n EOF\n\n?\n\n> + echo file2:world >non/expect.sub &&\n> + (\n> + GIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> + export GIT_CEILING_DIRECTORIES &&\n> + cd non/git &&\n> + git grep o >../actual.full &&\n> + test_cmp ../expect.full ../actual.full\n\nBroken &&-chain.\n\n> + cd sub &&\n> + git grep o >../../actual.sub &&\n> + test_cmp ../../expect.sub ../../actual.sub\n> + ) &&\n> +\n> + echo \".*o*\" >non/git/.gitignore &&\n> + (\n> + GIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> + export GIT_CEILING_DIRECTORIES &&\n> + cd non/git &&\n> + git grep --exclude-standard o >../actual.full &&\n> + test_cmp ../expect.full ../actual.full &&\n> +\n> + {\n> + echo \".gitignore:.*o*\"\n\nBroken &&-chain.\n\n> + cat ../expect.full\n> + } >../expect.with.ignored &&\n> + git grep --no-exclude o >../actual.full &&\n> + test_cmp ../expect.with.ignored ../actual.full\n> + )\n> +'\n"},{"id":"275634","messageId":"20160111111015.GA10612@hank","threadId":"41155","inReplyTo":"CACsJy8Bs3z0Gk3CjhyZGfOLA7R3pZQz7K5gk4BTytvYkZeyBtQ@mail.gmail.com","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T11:10:15Z","receivedAt":"2016-01-11T11:10:15Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/11, Duy Nguyen wrote:\n> On Sun, Jan 10, 2016 at 9:19 PM, Thomas Gummerer wrote:\n> > Currently when git grep is used outside of a git repository without the\n> > --no-index option git simply dies. For convenience, implicitly make git\n> > grep behave like git grep --no-index when it is called outside of a git\n> > repository.\n>\n> Should we have a line about this behavior in git-grep.txt, maybe the\n> description section?\n\nYes good point, the behavior change should definitely be documented.\n\n> I wonder if anybody wants the old behavior (e.g.\n> non-zero exit code when running outside a repo). If there is such a\n> case (*), we may need an option to revert it back (--no-no-index seems\n> ridiculous, maybe --use-index). The safest way though, is introduce a\n> new option like --use-index= then you can make\n> an grep alias with --use-index=optional.\n\nYou're right. I couldn't think of a reason why someone would rely on\nthe old behavior, but maybe I missed something. I like the idea of\nintroducing the --use-index=... option.\n\nHow should we handle priority between --no-index and --use-index,\nshould we just give --no-index priority if it is set and ignore the\nnew --use-index option, or is there some other way?\n\n> (*) I've been hitting really weird real-world use cases so I'm a bit paranoid..\n> --\n> Duy\n"},{"id":"275635","messageId":"20160111112944.GB10612@hank","threadId":"41155","inReplyTo":"CAPig+cRXy=CBZBTC7fU5wHA3d9Acqh_WYD54DmY1sHj7rob9Pw@mail.gmail.com","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T11:29:44Z","receivedAt":"2016-01-11T11:29:44Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/10, Eric Sunshine wrote:\n> On Sun, Jan 10, 2016 at 9:19 AM, Thomas Gummerer wrote:\n> > Currently when git grep is used outside of a git repository without the\n> > --no-index option git simply dies. For convenience, implicitly make git\n> > grep behave like git grep --no-index when it is called outside of a git\n> > repository.\n> >\n> > Signed-off-by: Thomas Gummerer \n> > ---\n> > diff --git a/builtin/grep.c b/builtin/grep.c\n> > @@ -19,6 +19,9 @@\n> > +#define GREP_NO_INDEX_EXPLICIT 1\n> > +#define GREP_NO_INDEX_IMPLICIT 2\n> > @@ -873,13 +881,21 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> > if (!show_in_pager && !opt.status_only)\n> > setup_pager();\n> >\n> > - if (no_index && (untracked || cached))\n> > - die(_(\"--cached or --untracked cannot be used with --no-index.\"));\n> > + if (untracked || cached) {\n> > + if (no_index == GREP_NO_INDEX_EXPLICIT)\n> > + die(_(\"--cached or --untracked cannot be used with --no-index.\"));\n> > + else if (no_index == GREP_NO_INDEX_IMPLICIT)\n>\n> Just below here when checking --untracked, you use a simple 'else'\n> rather than 'else if' to handle the other case of explicit vs\n> implicit. Why the inconsistency? Also, the ordering of 'if/else' arms\n> is opposite.\n\nThe reason is that I removed the no_index check from the surrounding\nif block, and grep should only die if no_index is set, while in the\nblock below it should also die if only untracked is set, but no_index\nisn't. I agree however that it's a bit confusing, so I'll just leave\nthe no_index check in the surrounding block and use the simple else\nhere, so the block ends up like this:\n\n\tif (no_index && (untracked || cached)) {\n\t\tif (no_index == GREP_NO_INDEX_IMPLICIT)\n\t\t\tdie(_(\"--cached or --untracked cannot be used outside a git repository.\"));\n\t\telse\n\t\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n\t}\n\n> > + die(_(\"--cached or --untracked cannot be used outside a git repository.\"));\n> > + }\n> >\n> > if (no_index || untracked) {\n> > int use_exclude = (opt_exclude < 0) ? !no_index : !!opt_exclude;\n> > - if (list.nr)\n> > - die(_(\"--no-index or --untracked cannot be used with revs.\"));\n> > + if (list.nr) {\n> > + if (no_index == GREP_NO_INDEX_IMPLICIT)\n> > + die(_(\"cannot use revs outside of a git repository.\"));\n> > + else\n> > + die(_(\"--no-index or --untracked cannot be used with revs.\"));\n> > + }\n> > diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\n> > @@ -821,6 +818,44 @@ test_expect_success 'outside of git repository' '\n> > +test_expect_success 'outside of git repository without --no-index' '\n> > + rm -fr non &&\n> > + mkdir -p non/git/sub &&\n> > + echo hello >non/git/file1 &&\n> > + echo world >non/git/sub/file2 &&\n> > + {\n> > + echo file1:hello &&\n> > + echo sub/file2:world\n> > + } >non/expect.full &&\n>\n> Isn't the above just a complicated way of saying:\n>\n> cat <<-\\EOF >non/expect.full &&\n> file:hello\n> sub/file2:world\n> EOF\n>\n> ?\n\nYes, the test case is mostly copy paste from the test case above it\nwhere it is written like this. Looking around in t7810, both the more\ncomplicated way as I used it, as well as the way you've written it are\nused, so I'll use the simpler way in the re-roll. Thanks.\n\n> > + echo file2:world >non/expect.sub &&\n> > + (\n> > + GIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> > + export GIT_CEILING_DIRECTORIES &&\n> > + cd non/git &&\n> > + git grep o >../actual.full &&\n> > + test_cmp ../expect.full ../actual.full\n>\n> Broken &&-chain.\n>\n> > + cd sub &&\n> > + git grep o >../../actual.sub &&\n> > + test_cmp ../../expect.sub ../../actual.sub\n> > + ) &&\n> > +\n> > + echo \".*o*\" >non/git/.gitignore &&\n> > + (\n> > + GIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> > + export GIT_CEILING_DIRECTORIES &&\n> > + cd non/git &&\n> > + git grep --exclude-standard o >../actual.full &&\n> > + test_cmp ../expect.full ../actual.full &&\n> > +\n> > + {\n> > + echo \".gitignore:.*o*\"\n>\n> Broken &&-chain.\n\nThanks for catching these two, again the test is mostly a copy paste,\nso the chain is broken in the test before this one as well, will add\nthe fix to my first cleanup patch for the other tests and fix these in\nthe re-roll.\n\nThanks for your review!\n\n> > + cat ../expect.full\n> > + } >../expect.with.ignored &&\n> > + git grep --no-exclude o >../actual.full &&\n> > + test_cmp ../expect.with.ignored ../actual.full\n> > + )\n> > +'\n"},{"id":"275660","messageId":"1452531648-16575-1-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452435597-12099-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v2 0/3] Introduce a --use-index command line argument in git grep","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T17:00:45Z","receivedAt":"2016-01-11T17:00:45Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"The first round is at $gmane/283619. Thanks to Duy and Eric for the reviews.\n\nI decided to go with Duy's suggestion, introducing a new --use-index\noption, instead of modifying the behaviour of git grep outside of a\ngit repository, as it seems to be the safer way to go.\n\nChanges since the last version:\n01/03: Fix the && chain in this test as well.\n03/03:\n - Introduced the new --no-index argument\n - fixed the test thanks to the suggestion of Eric\n - modified the test to use --use-index=optional and added a test that\n uses --use-index=never.\n - Fixed the if/else statements for the error messages to be more\n consistent.\n - Added documentation for the new option\n\nInterdiff below:\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 4a44d6d..90ff643 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -25,6 +25,7 @@ SYNOPSIS\n \t [-W | --function-context]\n \t [-f ] [-e] \n \t [--and|--or|--not|(|)|-e ...]\n+\t [--use-index=]\n \t [ [--[no-]exclude-standard] [--cached | --no-index | --untracked] | ...]\n \t [--] [...]\n \n@@ -66,6 +67,18 @@ OPTIONS\n --no-index::\n \tSearch files in the current directory that is not managed by Git.\n \n+--use-index::\n+\tSet the behavior of git grep when used in- or outside of a git\n+\trepository. If set to always, the command can only be used\n+\tinside of a git repository, and will fail if it is used\n+\toutside of a git repository. If set to never, it will work\n+\tboth inside and outside of a git repository, but will not use\n+\tany information in the index if used inside of a repository.\n+\tIf set to optional, it will work both inside and outside of a\n+\tgit repository, using the information available in the index\n+\tif used inside a repository. Always and optional can be\n+\toverridden by `--no-index`.\n+\n --untracked::\n \tIn addition to searching in the tracked files in the working\n \ttree, search also in untracked files.\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex a886af1..bb2add9 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -629,6 +629,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tint i;\n \tint dummy;\n \tint no_index = 0;\n+\tconst char *use_index = NULL;\n \tint pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;\n \n \tstruct option options[] = {\n@@ -637,6 +638,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT(0, \"no-index\", &no_index,\n \t\t\tN_(\"find in contents not managed by git\"),\n \t\t\tGREP_NO_INDEX_EXPLICIT),\n+\t\tOPT_STRING(0, \"use-index\", &use_index, N_(\"always|optional|never\"),\n+\t\t\t N_(\"options for using the index\")),\n \t\tOPT_BOOL(0, \"untracked\", &untracked,\n \t\t\tN_(\"search in both tracked and untracked files\")),\n \t\tOPT_SET_INT(0, \"exclude-standard\", &opt_exclude,\n@@ -759,14 +762,29 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n \n-\tif (!no_index && !startup_info->have_repository) {\n-\t\tint nongit = 0;\n+\tif (use_index) {\n+\t\tif (!strcmp(use_index, \"always\")) {\n+\t\t\t/* same as git grep without --use-index */\n+\t\t} else if (!strcmp(use_index, \"optional\")) {\n+\t\t\tif (!no_index && !startup_info->have_repository) {\n+\t\t\t\tint nongit = 0;\n \n-\t\tsetup_git_directory_gently(&nongit);\n-\t\tif (nongit)\n-\t\t\tno_index = GREP_NO_INDEX_IMPLICIT;\n+\t\t\t\tsetup_git_directory_gently(&nongit);\n+\t\t\t\tif (nongit)\n+\t\t\t\t\tno_index = GREP_NO_INDEX_IMPLICIT;\n+\t\t\t}\n+\t\t} else if (!strcmp(use_index, \"never\")) {\n+\t\t\tno_index = GREP_NO_INDEX_EXPLICIT;\n+\t\t} else {\n+\t\t\tdie(_(\"invalid option \\\"%s\\\" for --use-index, expecting\"\n+\t\t\t \" always, optional or never\"), use_index);\n+\t\t}\n \t}\n \n+\tif (!no_index && !startup_info->have_repository)\n+\t\t/* die the same way as if we did it at the beginning */\n+\t\tsetup_git_directory();\n+\n \t/*\n \t * skip a -- separator; we know it cannot be\n \t * separating revisions from pathnames if\n@@ -881,11 +899,13 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tif (!show_in_pager && !opt.status_only)\n \t\tsetup_pager();\n \n-\tif (untracked || cached) {\n-\t\tif (no_index == GREP_NO_INDEX_EXPLICIT)\n-\t\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n-\t\telse if (no_index == GREP_NO_INDEX_IMPLICIT)\n-\t\t\tdie(_(\"--cached or --untracked cannot be used outside a git repository.\"));\n+\tif (no_index && (untracked || cached)) {\n+\t\tif (no_index == GREP_NO_INDEX_IMPLICIT)\n+\t\t\tdie(_(\"--cached or --untracked cannot be used outside \"\n+\t\t\t \"of a git repository.\"));\n+\t\telse\n+\t\t\tdie(_(\"--cached or --untracked cannot be used with \"\n+\t\t\t \"--no-index or --no-index=never.\"));\n \t}\n \n \tif (no_index || untracked) {\n@@ -894,7 +914,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\tif (no_index == GREP_NO_INDEX_IMPLICIT)\n \t\t\t\tdie(_(\"cannot use revs outside of a git repository.\"));\n \t\t\telse\n-\t\t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n+\t\t\t\tdie(_(\"--no-index, --use-index=never or \"\n+\t\t\t\t \"--untracked cannot be used with revs.\"));\n \t\t}\n \t\thit = grep_directory(&opt, &pathspec, use_exclude);\n \t} else if (0 <= opt_exclude) {\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 4ba955d..9776f15 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -794,9 +794,11 @@ test_expect_success 'outside of git repository' '\n \t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n+\t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index o >../actual.full &&\n-\t\ttest_cmp ../expect.full ../actual.full\n+\t\ttest_cmp ../expect.full ../actual.full &&\n \t\tcd sub &&\n+\t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index o >../../actual.sub &&\n \t\ttest_cmp ../../expect.sub ../../actual.sub\n \t) &&\n@@ -806,11 +808,12 @@ test_expect_success 'outside of git repository' '\n \t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n+\t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index --exclude-standard o >../actual.full &&\n \t\ttest_cmp ../expect.full ../actual.full &&\n \n \t\t{\n-\t\t\techo \".gitignore:.*o*\"\n+\t\t\techo \".gitignore:.*o*\" &&\n \t\t\tcat ../expect.full\n \t\t} >../expect.with.ignored &&\n \t\tgit grep --no-index --no-exclude o >../actual.full &&\n@@ -818,24 +821,67 @@ test_expect_success 'outside of git repository' '\n \t)\n '\n \n-test_expect_success 'outside of git repository without --no-index' '\n+test_expect_success 'outside of git repository with --use-index=optional' '\n \trm -fr non &&\n \tmkdir -p non/git/sub &&\n \techo hello >non/git/file1 &&\n \techo world >non/git/sub/file2 &&\n-\t{\n-\t\techo file1:hello &&\n-\t\techo sub/file2:world\n-\t} >non/expect.full &&\n+\tcat <<-\\EOF >non/expect.full &&\n+\tfile1:hello\n+\tsub/file2:world\n+\tEOF\n+\techo file2:world >non/expect.sub &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=optional o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\t\tcd sub &&\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=optional o >../../actual.sub &&\n+\t\ttest_cmp ../../expect.sub ../../actual.sub\n+\t) &&\n+\n+\techo \".*o*\" >non/git/.gitignore &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=optional --exclude-standard o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\n+\t\t{\n+\t\t\techo \".gitignore:.*o*\" &&\n+\t\t\tcat ../expect.full\n+\t\t} >../expect.with.ignored &&\n+\t\tgit grep --use-index=optional --no-exclude o >../actual.full &&\n+\t\ttest_cmp ../expect.with.ignored ../actual.full\n+\t)\n+'\n+\n+test_expect_success 'outside of git repository with --use-index=never' '\n+\trm -fr non &&\n+\tmkdir -p non/git/sub &&\n+\techo hello >non/git/file1 &&\n+\techo world >non/git/sub/file2 &&\n+\tcat <<-\\EOF >non/expect.full &&\n+\tfile1:hello\n+\tsub/file2:world\n+\tEOF\n \techo file2:world >non/expect.sub &&\n \t(\n \t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n-\t\tgit grep o >../actual.full &&\n-\t\ttest_cmp ../expect.full ../actual.full\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=never o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n \t\tcd sub &&\n-\t\tgit grep o >../../actual.sub &&\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=never o >../../actual.sub &&\n \t\ttest_cmp ../../expect.sub ../../actual.sub\n \t) &&\n \n@@ -844,14 +890,15 @@ test_expect_success 'outside of git repository without --no-index' '\n \t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n-\t\tgit grep --exclude-standard o >../actual.full &&\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=never --exclude-standard o >../actual.full &&\n \t\ttest_cmp ../expect.full ../actual.full &&\n \n \t\t{\n-\t\t\techo \".gitignore:.*o*\"\n+\t\t\techo \".gitignore:.*o*\" &&\n \t\t\tcat ../expect.full\n \t\t} >../expect.with.ignored &&\n-\t\tgit grep --no-exclude o >../actual.full &&\n+\t\tgit grep --use-index=never --no-exclude o >../actual.full &&\n \t\ttest_cmp ../expect.with.ignored ../actual.full\n \t)\n '\n\nThomas Gummerer (3):\n t7810: correct --no-index test\n builtin/grep: rename use_index to no_index\n builtin/grep: introduce --use-index argument\n\n Documentation/git-grep.txt | 13 +++++++\n builtin/grep.c | 57 +++++++++++++++++++++++------\n t/t7810-grep.sh | 90 +++++++++++++++++++++++++++++++++++++++++++---\n 3 files changed, 146 insertions(+), 14 deletions(-)\n\n-- \n2.7.0.3.g214d8d0.dirty\n"},{"id":"275661","messageId":"1452531648-16575-2-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452531648-16575-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v2 1/3] t7810: correct --no-index test","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T17:00:46Z","receivedAt":"2016-01-11T17:00:46Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"GIT_CEILING_DIRECTORIES doesn't prevent chdir up into another directory\nwhile looking for a repository directory if it is equal to the current\ndirectory. Because of this, the test which claims to test the git grep\n--no-index command outside of a repository actually tests it inside of a\nrepository. The test_must_fail assertions still pass because the git\ngrep only looks at untracked files and therefore no file matches, but\nnot because it's run outside of a repository as it was originally\nintended.\n\nSet the GIT_CEILING_DIRECTORIES environment variable to the parent\ndirectory of the directory in which the git grep command is executed, to\nmake sure it is actually run outside of a git repository.\n\nIn addition the && chain was broken in a couple of places, fix that.\n\nSigned-off-by: Thomas Gummerer \n---\n t/t7810-grep.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 028ffe4..cc4b97d 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -791,12 +791,12 @@ test_expect_success 'outside of git repository' '\n \t} >non/expect.full &&\n \techo file2:world >non/expect.sub &&\n \t(\n-\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non/git\" &&\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n \t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index o >../actual.full &&\n-\t\ttest_cmp ../expect.full ../actual.full\n+\t\ttest_cmp ../expect.full ../actual.full &&\n \t\tcd sub &&\n \t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index o >../../actual.sub &&\n@@ -805,7 +805,7 @@ test_expect_success 'outside of git repository' '\n \n \techo \".*o*\" >non/git/.gitignore &&\n \t(\n-\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non/git\" &&\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n \t\ttest_must_fail git grep o &&\n@@ -813,7 +813,7 @@ test_expect_success 'outside of git repository' '\n \t\ttest_cmp ../expect.full ../actual.full &&\n \n \t\t{\n-\t\t\techo \".gitignore:.*o*\"\n+\t\t\techo \".gitignore:.*o*\" &&\n \t\t\tcat ../expect.full\n \t\t} >../expect.with.ignored &&\n \t\tgit grep --no-index --no-exclude o >../actual.full &&\n-- \n2.7.0.3.g214d8d0.dirty\n"},{"id":"275662","messageId":"1452531648-16575-3-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452531648-16575-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v2 2/3] builtin/grep: rename use_index to no_index","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T17:00:47Z","receivedAt":"2016-01-11T17:00:47Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Switch the OPT_NEGBIT argument to a OPT_BIT argument, and rename the\ncorresponding use_index variable to a no_index variable. This will make\nthe following step easier to follow. No functional changes intended.\n\nSigned-off-by: Thomas Gummerer \n---\n builtin/grep.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 4229cae..3a27bd5 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -625,14 +625,14 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tstruct string_list path_list = STRING_LIST_INIT_NODUP;\n \tint i;\n \tint dummy;\n-\tint use_index = 1;\n+\tint no_index = 0;\n \tint pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"cached\", &cached,\n \t\t\tN_(\"search in index instead of in the work tree\")),\n-\t\tOPT_NEGBIT(0, \"no-index\", &use_index,\n-\t\t\t N_(\"find in contents not managed by git\"), 1),\n+\t\tOPT_BIT(0, \"no-index\", &no_index,\n+\t\t\tN_(\"find in contents not managed by git\"), 1),\n \t\tOPT_BOOL(0, \"untracked\", &untracked,\n \t\t\tN_(\"search in both tracked and untracked files\")),\n \t\tOPT_SET_INT(0, \"exclude-standard\", &opt_exclude,\n@@ -755,7 +755,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n \n-\tif (use_index && !startup_info->have_repository)\n+\tif (!no_index && !startup_info->have_repository)\n \t\t/* die the same way as if we did it at the beginning */\n \t\tsetup_git_directory();\n \n@@ -873,11 +873,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tif (!show_in_pager && !opt.status_only)\n \t\tsetup_pager();\n \n-\tif (!use_index && (untracked || cached))\n+\tif (no_index && (untracked || cached))\n \t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n \n-\tif (!use_index || untracked) {\n-\t\tint use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude;\n+\tif (no_index || untracked) {\n+\t\tint use_exclude = (opt_exclude < 0) ? !no_index : !!opt_exclude;\n \t\tif (list.nr)\n \t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n \t\thit = grep_directory(&opt, &pathspec, use_exclude);\n-- \n2.7.0.3.g214d8d0.dirty\n"},{"id":"275663","messageId":"1452531648-16575-4-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452531648-16575-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v2 3/3] builtin/grep: introduce --use-index argument","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T17:00:48Z","receivedAt":"2016-01-11T17:00:48Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Currently when git grep is used outside of a git repository without the\n--no-index option git simply dies. Introduce a new --use-index command line\nargument, which can be set to always, optional, or never. If it is set\nto always, git grep continues to behave as it currently does. If set to\noptional, git grep will work normally if executed inside of a git\nrepository, and behave like git grep --no-index if used outside of a git\nrepository. If set to never, git grep will behave like git grep\n--no-index regardless of whether it is used inside or outside of a git\nrepository.\n\nHelped-by: Nguyễn Thái Ngọc Duy \nHelped-by: Eric Sunshine \nSigned-off-by: Thomas Gummerer \n---\n Documentation/git-grep.txt | 13 ++++++++\n builtin/grep.c | 47 +++++++++++++++++++++++---\n t/t7810-grep.sh | 82 ++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 137 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 4a44d6d..90ff643 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -25,6 +25,7 @@ SYNOPSIS\n \t [-W | --function-context]\n \t [-f ] [-e] \n \t [--and|--or|--not|(|)|-e ...]\n+\t [--use-index=]\n \t [ [--[no-]exclude-standard] [--cached | --no-index | --untracked] | ...]\n \t [--] [...]\n \n@@ -66,6 +67,18 @@ OPTIONS\n --no-index::\n \tSearch files in the current directory that is not managed by Git.\n \n+--use-index::\n+\tSet the behavior of git grep when used in- or outside of a git\n+\trepository. If set to always, the command can only be used\n+\tinside of a git repository, and will fail if it is used\n+\toutside of a git repository. If set to never, it will work\n+\tboth inside and outside of a git repository, but will not use\n+\tany information in the index if used inside of a repository.\n+\tIf set to optional, it will work both inside and outside of a\n+\tgit repository, using the information available in the index\n+\tif used inside a repository. Always and optional can be\n+\toverridden by `--no-index`.\n+\n --untracked::\n \tIn addition to searching in the tracked files in the working\n \ttree, search also in untracked files.\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 3a27bd5..bb2add9 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -19,6 +19,9 @@\n #include \"dir.h\"\n #include \"pathspec.h\"\n \n+#define GREP_NO_INDEX_EXPLICIT 1\n+#define GREP_NO_INDEX_IMPLICIT 2\n+\n static char const * const grep_usage[] = {\n \tN_(\"git grep [] [-e] [...] [[--] ...]\"),\n \tNULL\n@@ -626,13 +629,17 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tint i;\n \tint dummy;\n \tint no_index = 0;\n+\tconst char *use_index = NULL;\n \tint pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"cached\", &cached,\n \t\t\tN_(\"search in index instead of in the work tree\")),\n \t\tOPT_BIT(0, \"no-index\", &no_index,\n-\t\t\tN_(\"find in contents not managed by git\"), 1),\n+\t\t\tN_(\"find in contents not managed by git\"),\n+\t\t\tGREP_NO_INDEX_EXPLICIT),\n+\t\tOPT_STRING(0, \"use-index\", &use_index, N_(\"always|optional|never\"),\n+\t\t\t N_(\"options for using the index\")),\n \t\tOPT_BOOL(0, \"untracked\", &untracked,\n \t\t\tN_(\"search in both tracked and untracked files\")),\n \t\tOPT_SET_INT(0, \"exclude-standard\", &opt_exclude,\n@@ -755,6 +762,25 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n \n+\tif (use_index) {\n+\t\tif (!strcmp(use_index, \"always\")) {\n+\t\t\t/* same as git grep without --use-index */\n+\t\t} else if (!strcmp(use_index, \"optional\")) {\n+\t\t\tif (!no_index && !startup_info->have_repository) {\n+\t\t\t\tint nongit = 0;\n+\n+\t\t\t\tsetup_git_directory_gently(&nongit);\n+\t\t\t\tif (nongit)\n+\t\t\t\t\tno_index = GREP_NO_INDEX_IMPLICIT;\n+\t\t\t}\n+\t\t} else if (!strcmp(use_index, \"never\")) {\n+\t\t\tno_index = GREP_NO_INDEX_EXPLICIT;\n+\t\t} else {\n+\t\t\tdie(_(\"invalid option \\\"%s\\\" for --use-index, expecting\"\n+\t\t\t \" always, optional or never\"), use_index);\n+\t\t}\n+\t}\n+\n \tif (!no_index && !startup_info->have_repository)\n \t\t/* die the same way as if we did it at the beginning */\n \t\tsetup_git_directory();\n@@ -873,13 +899,24 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tif (!show_in_pager && !opt.status_only)\n \t\tsetup_pager();\n \n-\tif (no_index && (untracked || cached))\n-\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n+\tif (no_index && (untracked || cached)) {\n+\t\tif (no_index == GREP_NO_INDEX_IMPLICIT)\n+\t\t\tdie(_(\"--cached or --untracked cannot be used outside \"\n+\t\t\t \"of a git repository.\"));\n+\t\telse\n+\t\t\tdie(_(\"--cached or --untracked cannot be used with \"\n+\t\t\t \"--no-index or --no-index=never.\"));\n+\t}\n \n \tif (no_index || untracked) {\n \t\tint use_exclude = (opt_exclude < 0) ? !no_index : !!opt_exclude;\n-\t\tif (list.nr)\n-\t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n+\t\tif (list.nr) {\n+\t\t\tif (no_index == GREP_NO_INDEX_IMPLICIT)\n+\t\t\t\tdie(_(\"cannot use revs outside of a git repository.\"));\n+\t\t\telse\n+\t\t\t\tdie(_(\"--no-index, --use-index=never or \"\n+\t\t\t\t \"--untracked cannot be used with revs.\"));\n+\t\t}\n \t\thit = grep_directory(&opt, &pathspec, use_exclude);\n \t} else if (0 <= opt_exclude) {\n \t\tdie(_(\"--[no-]exclude-standard cannot be used for tracked contents.\"));\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex cc4b97d..9776f15 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -821,6 +821,88 @@ test_expect_success 'outside of git repository' '\n \t)\n '\n \n+test_expect_success 'outside of git repository with --use-index=optional' '\n+\trm -fr non &&\n+\tmkdir -p non/git/sub &&\n+\techo hello >non/git/file1 &&\n+\techo world >non/git/sub/file2 &&\n+\tcat <<-\\EOF >non/expect.full &&\n+\tfile1:hello\n+\tsub/file2:world\n+\tEOF\n+\techo file2:world >non/expect.sub &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=optional o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\t\tcd sub &&\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=optional o >../../actual.sub &&\n+\t\ttest_cmp ../../expect.sub ../../actual.sub\n+\t) &&\n+\n+\techo \".*o*\" >non/git/.gitignore &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=optional --exclude-standard o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\n+\t\t{\n+\t\t\techo \".gitignore:.*o*\" &&\n+\t\t\tcat ../expect.full\n+\t\t} >../expect.with.ignored &&\n+\t\tgit grep --use-index=optional --no-exclude o >../actual.full &&\n+\t\ttest_cmp ../expect.with.ignored ../actual.full\n+\t)\n+'\n+\n+test_expect_success 'outside of git repository with --use-index=never' '\n+\trm -fr non &&\n+\tmkdir -p non/git/sub &&\n+\techo hello >non/git/file1 &&\n+\techo world >non/git/sub/file2 &&\n+\tcat <<-\\EOF >non/expect.full &&\n+\tfile1:hello\n+\tsub/file2:world\n+\tEOF\n+\techo file2:world >non/expect.sub &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=never o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\t\tcd sub &&\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=never o >../../actual.sub &&\n+\t\ttest_cmp ../../expect.sub ../../actual.sub\n+\t) &&\n+\n+\techo \".*o*\" >non/git/.gitignore &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\ttest_must_fail git grep --use-index=always o &&\n+\t\tgit grep --use-index=never --exclude-standard o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\n+\t\t{\n+\t\t\techo \".gitignore:.*o*\" &&\n+\t\t\tcat ../expect.full\n+\t\t} >../expect.with.ignored &&\n+\t\tgit grep --use-index=never --no-exclude o >../actual.full &&\n+\t\ttest_cmp ../expect.with.ignored ../actual.full\n+\t)\n+'\n+\n test_expect_success 'inside git repository but with --no-index' '\n \trm -fr is &&\n \tmkdir -p is/git/sub &&\n-- \n2.7.0.3.g214d8d0.dirty\n"},{"id":"275668","messageId":"xmqqlh7wxc0y.fsf@gitster.mtv.corp.google.com","threadId":"41155","inReplyTo":"20160111111015.GA10612@hank","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-11T17:26:53Z","receivedAt":"2016-01-11T17:26:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> On 01/11, Duy Nguyen wrote:\n>> On Sun, Jan 10, 2016 at 9:19 PM, Thomas Gummerer wrote:\n>> > Currently when git grep is used outside of a git repository without the\n>> > --no-index option git simply dies. For convenience, implicitly make git\n>> > grep behave like git grep --no-index when it is called outside of a git\n>> > repository.\n>>\n>> Should we have a line about this behavior in git-grep.txt, maybe the\n>> description section?\n>\n> Yes good point, the behavior change should definitely be documented.\n>\n>> I wonder if anybody wants the old behavior (e.g.\n>> non-zero exit code when running outside a repo). If there is such a\n>> case (*), we may need an option to revert it back (--no-no-index seems\n>> ridiculous, maybe --use-index). The safest way though, is introduce a\n>> new option like --use-index= then you can make\n>> an grep alias with --use-index=optional.\n>\n> You're right. I couldn't think of a reason why someone would rely on\n> the old behavior, but maybe I missed something. I like the idea of\n> introducing the --use-index=... option.\n\nI don't like that, though ;-)\n\n\"We run 'git grep' in random places and relied on it to fail when\nrun in somewhere not under control of Git.\" feels so flawed at\nmultiple levels that I doubt it deserves to be kept working. For\none thing, \"git grep\" is not the way to tell something is under\ncontrol of Git (rev-parse would be a better thing for scriptor to\nuse). For another, how would such a script tell between \"not a\ngit repository\" and there was no hits?\n\nSo I do agree that automatic fallback needs to be documented and\nadvertised as a feature (or even a bugfix), I do not think we want\nto add knobs to keep such a broken script working.\n\n> How should we handle priority between --no-index and --use-index,\n> should we just give --no-index priority if it is set and ignore the\n> new --use-index option, or is there some other way?\n>\n>> (*) I've been hitting really weird real-world use cases so I'm a bit paranoid..\n>> --\n>> Duy\n"},{"id":"275669","messageId":"xmqqh9ikxbv7.fsf@gitster.mtv.corp.google.com","threadId":"41155","inReplyTo":"1452435597-12099-4-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-11T17:30:20Z","receivedAt":"2016-01-11T17:30:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> Currently when git grep is used outside of a git repository without the\n> --no-index option git simply dies. For convenience, implicitly make git\n> grep behave like git grep --no-index when it is called outside of a git\n> repository.\n>\n> Signed-off-by: Thomas Gummerer \n> ---\n> builtin/grep.c | 32 ++++++++++++++++++++++++--------\n> t/t7810-grep.sh | 41 ++++++++++++++++++++++++++++++++++++++---\n> 2 files changed, 62 insertions(+), 11 deletions(-)\n>\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 3a27bd5..a886af1 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -19,6 +19,9 @@\n> #include \"dir.h\"\n> #include \"pathspec.h\"\n> \n> +#define GREP_NO_INDEX_EXPLICIT 1\n> +#define GREP_NO_INDEX_IMPLICIT 2\n\nI am not sure this is the best way to do this. For things like\nthis, the usual pattern is to initialize \"no_index\" to an \"unknown\"\nvalue, allow \"--no-index\" to toggle it to true (by the way, I think\nwe should reject \"--no-no-index\", but that is a separate topic), and\nthen after command line parsing finishes, tweak the no_index if it\nis still \"unknown\".\n\n> static char const * const grep_usage[] = {\n> \tN_(\"git grep [] [-e] [...] [[--] ...]\"),\n> \tNULL\n> @@ -632,7 +635,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BOOL(0, \"cached\", &cached,\n> \t\t\tN_(\"search in index instead of in the work tree\")),\n> \t\tOPT_BIT(0, \"no-index\", &no_index,\n> -\t\t\tN_(\"find in contents not managed by git\"), 1),\n> +\t\t\tN_(\"find in contents not managed by git\"),\n> +\t\t\tGREP_NO_INDEX_EXPLICIT),\n> \t\tOPT_BOOL(0, \"untracked\", &untracked,\n> \t\t\tN_(\"search in both tracked and untracked files\")),\n> \t\tOPT_SET_INT(0, \"exclude-standard\", &opt_exclude,\n> @@ -755,9 +759,13 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n> \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n> \n> -\tif (!no_index && !startup_info->have_repository)\n> -\t\t/* die the same way as if we did it at the beginning */\n> -\t\tsetup_git_directory();\n> +\tif (!no_index && !startup_info->have_repository) {\n> +\t\tint nongit = 0;\n> +\n> +\t\tsetup_git_directory_gently(&nongit);\n> +\t\tif (nongit)\n> +\t\t\tno_index = GREP_NO_INDEX_IMPLICIT;\n> +\t}\n> \n> \t/*\n> \t * skip a -- separator; we know it cannot be\n> @@ -873,13 +881,21 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \tif (!show_in_pager && !opt.status_only)\n> \t\tsetup_pager();\n> \n> -\tif (no_index && (untracked || cached))\n> -\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n> +\tif (untracked || cached) {\n> +\t\tif (no_index == GREP_NO_INDEX_EXPLICIT)\n> +\t\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n> +\t\telse if (no_index == GREP_NO_INDEX_IMPLICIT)\n> +\t\t\tdie(_(\"--cached or --untracked cannot be used outside a git repository.\"));\n> +\t}\n> \n> \tif (no_index || untracked) {\n> \t\tint use_exclude = (opt_exclude < 0) ? !no_index : !!opt_exclude;\n> -\t\tif (list.nr)\n> -\t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n> +\t\tif (list.nr) {\n> +\t\t\tif (no_index == GREP_NO_INDEX_IMPLICIT)\n> +\t\t\t\tdie(_(\"cannot use revs outside of a git repository.\"));\n> +\t\t\telse\n> +\t\t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n> +\t\t}\n> \t\thit = grep_directory(&opt, &pathspec, use_exclude);\n> \t} else if (0 <= opt_exclude) {\n> \t\tdie(_(\"--[no-]exclude-standard cannot be used for tracked contents.\"));\n> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\n> index ab94716..4ba955d 100755\n> --- a/t/t7810-grep.sh\n> +++ b/t/t7810-grep.sh\n> @@ -794,11 +794,9 @@ test_expect_success 'outside of git repository' '\n> \t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> \t\texport GIT_CEILING_DIRECTORIES &&\n> \t\tcd non/git &&\n> -\t\ttest_must_fail git grep o &&\n> \t\tgit grep --no-index o >../actual.full &&\n> \t\ttest_cmp ../expect.full ../actual.full\n> \t\tcd sub &&\n> -\t\ttest_must_fail git grep o &&\n> \t\tgit grep --no-index o >../../actual.sub &&\n> \t\ttest_cmp ../../expect.sub ../../actual.sub\n> \t) &&\n> @@ -808,7 +806,6 @@ test_expect_success 'outside of git repository' '\n> \t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> \t\texport GIT_CEILING_DIRECTORIES &&\n> \t\tcd non/git &&\n> -\t\ttest_must_fail git grep o &&\n> \t\tgit grep --no-index --exclude-standard o >../actual.full &&\n> \t\ttest_cmp ../expect.full ../actual.full &&\n> \n> @@ -821,6 +818,44 @@ test_expect_success 'outside of git repository' '\n> \t)\n> '\n> \n> +test_expect_success 'outside of git repository without --no-index' '\n> +\trm -fr non &&\n> +\tmkdir -p non/git/sub &&\n> +\techo hello >non/git/file1 &&\n> +\techo world >non/git/sub/file2 &&\n> +\t{\n> +\t\techo file1:hello &&\n> +\t\techo sub/file2:world\n> +\t} >non/expect.full &&\n> +\techo file2:world >non/expect.sub &&\n> +\t(\n> +\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> +\t\texport GIT_CEILING_DIRECTORIES &&\n> +\t\tcd non/git &&\n> +\t\tgit grep o >../actual.full &&\n> +\t\ttest_cmp ../expect.full ../actual.full\n> +\t\tcd sub &&\n> +\t\tgit grep o >../../actual.sub &&\n> +\t\ttest_cmp ../../expect.sub ../../actual.sub\n> +\t) &&\n> +\n> +\techo \".*o*\" >non/git/.gitignore &&\n> +\t(\n> +\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> +\t\texport GIT_CEILING_DIRECTORIES &&\n> +\t\tcd non/git &&\n> +\t\tgit grep --exclude-standard o >../actual.full &&\n> +\t\ttest_cmp ../expect.full ../actual.full &&\n> +\n> +\t\t{\n> +\t\t\techo \".gitignore:.*o*\"\n> +\t\t\tcat ../expect.full\n> +\t\t} >../expect.with.ignored &&\n> +\t\tgit grep --no-exclude o >../actual.full &&\n> +\t\ttest_cmp ../expect.with.ignored ../actual.full\n> +\t)\n> +'\n> +\n> test_expect_success 'inside git repository but with --no-index' '\n> \trm -fr is &&\n> \tmkdir -p is/git/sub &&\n"},{"id":"275671","messageId":"xmqqd1t8xbq2.fsf@gitster.mtv.corp.google.com","threadId":"41155","inReplyTo":"1452435597-12099-3-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH 2/3] builtin/grep: rename use_index to no_index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-11T17:33:25Z","receivedAt":"2016-01-11T17:33:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n> Switch the OPT_NEGBIT argument to a OPT_BIT argument, and rename the\n> corresponding use_index variable to a no_index variable. This will make\n> the following step easier to follow. No functional changes intended.\n>\n> Signed-off-by: Thomas Gummerer \n> ---\n> builtin/grep.c | 14 +++++++-------\n> 1 file changed, 7 insertions(+), 7 deletions(-)\n\nThis makes the resulting code full of double negations, I am\nafraid. Please do not do this.\n\nI am open to turning \"use_index\" to a tristate, initialized to -1\n(unknown), set to 0 with OPT_SET_INT(\"no-index\"), and optionally set\nto 1 with OPT_SET_INT(\"use-index\"). Then after parsing the command\nline argument, if use_index is still -1 (i.e. no command line option),\nset it to 1 when we can use the index, or 0 (the new feature you\nwant to add with 3/3).\n\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 4229cae..3a27bd5 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -625,14 +625,14 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \tstruct string_list path_list = STRING_LIST_INIT_NODUP;\n> \tint i;\n> \tint dummy;\n> -\tint use_index = 1;\n> +\tint no_index = 0;\n> \tint pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;\n> \n> \tstruct option options[] = {\n> \t\tOPT_BOOL(0, \"cached\", &cached,\n> \t\t\tN_(\"search in index instead of in the work tree\")),\n> -\t\tOPT_NEGBIT(0, \"no-index\", &use_index,\n> -\t\t\t N_(\"find in contents not managed by git\"), 1),\n> +\t\tOPT_BIT(0, \"no-index\", &no_index,\n> +\t\t\tN_(\"find in contents not managed by git\"), 1),\n> \t\tOPT_BOOL(0, \"untracked\", &untracked,\n> \t\t\tN_(\"search in both tracked and untracked files\")),\n> \t\tOPT_SET_INT(0, \"exclude-standard\", &opt_exclude,\n> @@ -755,7 +755,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n> \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n> \n> -\tif (use_index && !startup_info->have_repository)\n> +\tif (!no_index && !startup_info->have_repository)\n> \t\t/* die the same way as if we did it at the beginning */\n> \t\tsetup_git_directory();\n> \n> @@ -873,11 +873,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \tif (!show_in_pager && !opt.status_only)\n> \t\tsetup_pager();\n> \n> -\tif (!use_index && (untracked || cached))\n> +\tif (no_index && (untracked || cached))\n> \t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n> \n> -\tif (!use_index || untracked) {\n> -\t\tint use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude;\n> +\tif (no_index || untracked) {\n> +\t\tint use_exclude = (opt_exclude < 0) ? !no_index : !!opt_exclude;\n> \t\tif (list.nr)\n> \t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n> \t\thit = grep_directory(&opt, &pathspec, use_exclude);\n"},{"id":"275672","messageId":"20160111174817.GC10612@hank","threadId":"41155","inReplyTo":"xmqqlh7wxc0y.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T17:48:17Z","receivedAt":"2016-01-11T17:48:17Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/11, Junio C Hamano wrote:\n> Thomas Gummerer writes:\n>\n> > On 01/11, Duy Nguyen wrote:\n> >> On Sun, Jan 10, 2016 at 9:19 PM, Thomas Gummerer wrote:\n> >> > Currently when git grep is used outside of a git repository without the\n> >> > --no-index option git simply dies. For convenience, implicitly make git\n> >> > grep behave like git grep --no-index when it is called outside of a git\n> >> > repository.\n> >>\n> >> Should we have a line about this behavior in git-grep.txt, maybe the\n> >> description section?\n> >\n> > Yes good point, the behavior change should definitely be documented.\n> >\n> >> I wonder if anybody wants the old behavior (e.g.\n> >> non-zero exit code when running outside a repo). If there is such a\n> >> case (*), we may need an option to revert it back (--no-no-index seems\n> >> ridiculous, maybe --use-index). The safest way though, is introduce a\n> >> new option like --use-index= then you can make\n> >> an grep alias with --use-index=optional.\n> >\n> > You're right. I couldn't think of a reason why someone would rely on\n> > the old behavior, but maybe I missed something. I like the idea of\n> > introducing the --use-index=... option.\n>\n> I don't like that, though ;-)\n>\n> \"We run 'git grep' in random places and relied on it to fail when\n> run in somewhere not under control of Git.\" feels so flawed at\n> multiple levels that I doubt it deserves to be kept working. For\n> one thing, \"git grep\" is not the way to tell something is under\n> control of Git (rev-parse would be a better thing for scriptor to\n> use). For another, how would such a script tell between \"not a\n> git repository\" and there was no hits?\n\nI agree that scripts don't deserve to be kept working in that case.\nWhat about a user though who accidentally runs git grep outside of a\nrepository, and is usually warned by git failing quickly, whereas with\nthe changed behavior some time might go by until the user realizes the\nerror. Not sure if we want to support this use case or not?\n\n> So I do agree that automatic fallback needs to be documented and\n> advertised as a feature (or even a bugfix), I do not think we want\n> to add knobs to keep such a broken script working.\n>\n> > How should we handle priority between --no-index and --use-index,\n> > should we just give --no-index priority if it is set and ignore the\n> > new --use-index option, or is there some other way?\n> >\n> >> (*) I've been hitting really weird real-world use cases so I'm a bit paranoid..\n> >> --\n> >> Duy\n\n--\nThomas Gummerer\n"},{"id":"275675","messageId":"20160111175919.GA11724@sigill.intra.peff.net","threadId":"41155","inReplyTo":"20160111174817.GC10612@hank","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-11T17:59:19Z","receivedAt":"2016-01-11T17:59:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 11, 2016 at 06:48:17PM +0100, Thomas Gummerer wrote:\n\n> > \"We run 'git grep' in random places and relied on it to fail when\n> > run in somewhere not under control of Git.\" feels so flawed at\n> > multiple levels that I doubt it deserves to be kept working. For\n> > one thing, \"git grep\" is not the way to tell something is under\n> > control of Git (rev-parse would be a better thing for scriptor to\n> > use). For another, how would such a script tell between \"not a\n> > git repository\" and there was no hits?\n> \n> I agree that scripts don't deserve to be kept working in that case.\n> What about a user though who accidentally runs git grep outside of a\n> repository, and is usually warned by git failing quickly, whereas with\n> the changed behavior some time might go by until the user realizes the\n> error. Not sure if we want to support this use case or not?\n\nYeah, I don't think git would be _wrong_ here, but I could certainly see\nit being annoying. Several times a week I probably run `git grep` in my\nhome directory, and after seeing its error, realize \"oops, I meant to\n`cd git`\".\n\nHaving it spew nonsense results, and/or appear to hang while it\nliterally reads every file on my disk would be at least a minor\nannoyance.\n\nBut I don't think any kind of command-line flag would help that; I'm not\ngoing to start typing \"git grep --use-index=never\" for every invocation.\nI think the only sensible mitigation would be a config option, so that\npeople who rarely use `--no-index` (and are OK with having to specify\nit) do not get punished by false positives.\n\nI dunno. Maybe I would find the new behavior so useful I would be OK\nwith the occasional false-positive. But when we make a release with the\nnew behavior and somebody _does_ complain, it sure would be nice not to\nhave to say \"deal with it; it's the new behavior and there is no escape\nhatch\".\n\n-Peff\n"},{"id":"275688","messageId":"xmqqvb70vu6e.fsf@gitster.mtv.corp.google.com","threadId":"41155","inReplyTo":"20160111175919.GA11724@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-11T18:37:45Z","receivedAt":"2016-01-11T18:37:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Jan 11, 2016 at 06:48:17PM +0100, Thomas Gummerer wrote:\n>\n>> What about a user though who accidentally runs git grep outside of a\n>> repository, and is usually warned by git failing quickly, whereas with\n>> the changed behavior some time might go by until the user realizes the\n>> error. Not sure if we want to support this use case or not?\n>\n> Yeah, I don't think git would be _wrong_ here, but I could certainly see\n> it being annoying. Several times a week I probably run `git grep` in my\n> home directory, and after seeing its error, realize \"oops, I meant to\n> `cd git`\".\n> ...\n> ..., it sure would be nice not to\n> have to say \"deal with it; it's the new behavior and there is no escape\n> hatch\".\n\nAhh, you two are absolutely right.\n\nI assumed people (after making their new year's resolutions) will be\nperfect this year and from now on, and I totally forgot about the\ncase where somebody runs it by mistake.\n\n\"grep.fallbacktoNoindex = true/false\" that is by default 'false'\nwould be our usual approach, then.\n\nThanks for pointing it out.\n"},{"id":"275689","messageId":"20160111192844.GD10612@hank","threadId":"41155","inReplyTo":"xmqqh9ikxbv7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T19:28:44Z","receivedAt":"2016-01-11T19:28:44Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/11, Junio C Hamano wrote:\n> Thomas Gummerer writes:\n>\n> > Currently when git grep is used outside of a git repository without the\n> > --no-index option git simply dies. For convenience, implicitly make git\n> > grep behave like git grep --no-index when it is called outside of a git\n> > repository.\n> >\n> > Signed-off-by: Thomas Gummerer \n> > ---\n> > builtin/grep.c | 32 ++++++++++++++++++++++++--------\n> > t/t7810-grep.sh | 41 ++++++++++++++++++++++++++++++++++++++---\n> > 2 files changed, 62 insertions(+), 11 deletions(-)\n> >\n> > diff --git a/builtin/grep.c b/builtin/grep.c\n> > index 3a27bd5..a886af1 100644\n> > --- a/builtin/grep.c\n> > +++ b/builtin/grep.c\n> > @@ -19,6 +19,9 @@\n> > #include \"dir.h\"\n> > #include \"pathspec.h\"\n> >\n> > +#define GREP_NO_INDEX_EXPLICIT 1\n> > +#define GREP_NO_INDEX_IMPLICIT 2\n>\n> I am not sure this is the best way to do this. For things like\n> this, the usual pattern is to initialize \"no_index\" to an \"unknown\"\n> value, allow \"--no-index\" to toggle it to true (by the way, I think\n> we should reject \"--no-no-index\", but that is a separate topic), and\n> then after command line parsing finishes, tweak the no_index if it\n> is still \"unknown\".\n\nThe reason for this (and the change in 02/03) is so we can distinguish\nwhether there is an explicit no-index or not for the error messages.\nI think it would be okay to have more generic error messages\n(\"--cached or --untracked cannot be used without index\" and\n\"--untracked or no index mode cannot be used with revs\"). What do you\nthink?\n\n> > static char const * const grep_usage[] = {\n> > \tN_(\"git grep [] [-e] [...] [[--] ...]\"),\n> > \tNULL\n> > @@ -632,7 +635,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> > \t\tOPT_BOOL(0, \"cached\", &cached,\n> > \t\t\tN_(\"search in index instead of in the work tree\")),\n> > \t\tOPT_BIT(0, \"no-index\", &no_index,\n> > -\t\t\tN_(\"find in contents not managed by git\"), 1),\n> > +\t\t\tN_(\"find in contents not managed by git\"),\n> > +\t\t\tGREP_NO_INDEX_EXPLICIT),\n> > \t\tOPT_BOOL(0, \"untracked\", &untracked,\n> > \t\t\tN_(\"search in both tracked and untracked files\")),\n> > \t\tOPT_SET_INT(0, \"exclude-standard\", &opt_exclude,\n> > @@ -755,9 +759,13 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> > \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n> > \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n> >\n> > -\tif (!no_index && !startup_info->have_repository)\n> > -\t\t/* die the same way as if we did it at the beginning */\n> > -\t\tsetup_git_directory();\n> > +\tif (!no_index && !startup_info->have_repository) {\n> > +\t\tint nongit = 0;\n> > +\n> > +\t\tsetup_git_directory_gently(&nongit);\n> > +\t\tif (nongit)\n> > +\t\t\tno_index = GREP_NO_INDEX_IMPLICIT;\n> > +\t}\n> >\n> > \t/*\n> > \t * skip a -- separator; we know it cannot be\n> > @@ -873,13 +881,21 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> > \tif (!show_in_pager && !opt.status_only)\n> > \t\tsetup_pager();\n> >\n> > -\tif (no_index && (untracked || cached))\n> > -\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n> > +\tif (untracked || cached) {\n> > +\t\tif (no_index == GREP_NO_INDEX_EXPLICIT)\n> > +\t\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n> > +\t\telse if (no_index == GREP_NO_INDEX_IMPLICIT)\n> > +\t\t\tdie(_(\"--cached or --untracked cannot be used outside a git repository.\"));\n> > +\t}\n> >\n> > \tif (no_index || untracked) {\n> > \t\tint use_exclude = (opt_exclude < 0) ? !no_index : !!opt_exclude;\n> > -\t\tif (list.nr)\n> > -\t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n> > +\t\tif (list.nr) {\n> > +\t\t\tif (no_index == GREP_NO_INDEX_IMPLICIT)\n> > +\t\t\t\tdie(_(\"cannot use revs outside of a git repository.\"));\n> > +\t\t\telse\n> > +\t\t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n> > +\t\t}\n> > \t\thit = grep_directory(&opt, &pathspec, use_exclude);\n> > \t} else if (0 <= opt_exclude) {\n> > \t\tdie(_(\"--[no-]exclude-standard cannot be used for tracked contents.\"));\n> > diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\n> > index ab94716..4ba955d 100755\n> > --- a/t/t7810-grep.sh\n> > +++ b/t/t7810-grep.sh\n> > @@ -794,11 +794,9 @@ test_expect_success 'outside of git repository' '\n> > \t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> > \t\texport GIT_CEILING_DIRECTORIES &&\n> > \t\tcd non/git &&\n> > -\t\ttest_must_fail git grep o &&\n> > \t\tgit grep --no-index o >../actual.full &&\n> > \t\ttest_cmp ../expect.full ../actual.full\n> > \t\tcd sub &&\n> > -\t\ttest_must_fail git grep o &&\n> > \t\tgit grep --no-index o >../../actual.sub &&\n> > \t\ttest_cmp ../../expect.sub ../../actual.sub\n> > \t) &&\n> > @@ -808,7 +806,6 @@ test_expect_success 'outside of git repository' '\n> > \t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> > \t\texport GIT_CEILING_DIRECTORIES &&\n> > \t\tcd non/git &&\n> > -\t\ttest_must_fail git grep o &&\n> > \t\tgit grep --no-index --exclude-standard o >../actual.full &&\n> > \t\ttest_cmp ../expect.full ../actual.full &&\n> >\n> > @@ -821,6 +818,44 @@ test_expect_success 'outside of git repository' '\n> > \t)\n> > '\n> >\n> > +test_expect_success 'outside of git repository without --no-index' '\n> > +\trm -fr non &&\n> > +\tmkdir -p non/git/sub &&\n> > +\techo hello >non/git/file1 &&\n> > +\techo world >non/git/sub/file2 &&\n> > +\t{\n> > +\t\techo file1:hello &&\n> > +\t\techo sub/file2:world\n> > +\t} >non/expect.full &&\n> > +\techo file2:world >non/expect.sub &&\n> > +\t(\n> > +\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> > +\t\texport GIT_CEILING_DIRECTORIES &&\n> > +\t\tcd non/git &&\n> > +\t\tgit grep o >../actual.full &&\n> > +\t\ttest_cmp ../expect.full ../actual.full\n> > +\t\tcd sub &&\n> > +\t\tgit grep o >../../actual.sub &&\n> > +\t\ttest_cmp ../../expect.sub ../../actual.sub\n> > +\t) &&\n> > +\n> > +\techo \".*o*\" >non/git/.gitignore &&\n> > +\t(\n> > +\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n> > +\t\texport GIT_CEILING_DIRECTORIES &&\n> > +\t\tcd non/git &&\n> > +\t\tgit grep --exclude-standard o >../actual.full &&\n> > +\t\ttest_cmp ../expect.full ../actual.full &&\n> > +\n> > +\t\t{\n> > +\t\t\techo \".gitignore:.*o*\"\n> > +\t\t\tcat ../expect.full\n> > +\t\t} >../expect.with.ignored &&\n> > +\t\tgit grep --no-exclude o >../actual.full &&\n> > +\t\ttest_cmp ../expect.with.ignored ../actual.full\n> > +\t)\n> > +'\n> > +\n> > test_expect_success 'inside git repository but with --no-index' '\n> > \trm -fr is &&\n> > \tmkdir -p is/git/sub &&\n\n--\nThomas Gummerer\n"},{"id":"275691","messageId":"xmqqmvsbx62d.fsf@gitster.mtv.corp.google.com","threadId":"41155","inReplyTo":"20160111192844.GD10612@hank","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-11T19:35:38Z","receivedAt":"2016-01-11T19:35:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Gummerer writes:\n\n>> > +#define GREP_NO_INDEX_EXPLICIT 1\n>> > +#define GREP_NO_INDEX_IMPLICIT 2\n>>\n>> I am not sure this is the best way to do this. For things like\n>> this, the usual pattern is to initialize \"no_index\" to an \"unknown\"\n>> value, allow \"--no-index\" to toggle it to true (by the way, I think\n>> we should reject \"--no-no-index\", but that is a separate topic), and\n>> then after command line parsing finishes, tweak the no_index if it\n>> is still \"unknown\".\n>\n> The reason for this (and the change in 02/03) is so we can distinguish\n> whether there is an explicit no-index or not for the error messages.\n> I think it would be okay to have more generic error messages\n> (\"--cached or --untracked cannot be used without index\" and\n> \"--untracked or no index mode cannot be used with revs\"). What do you\n> think?\n\nI can understand that you need three states (--no-index given\nexplicitly from the command line, we fall back to --no-index when we\nfound we are in a diretory not under control by Git, and we do want\nto use the index) to be able to give different messages between the\nfirst two cases.\n\nThe usual way we do that is by making the variable tristate (which I\noutlined in the previous message, initialize use_index to -1\n\"unspecified\" and then fix it up when it is left unspecified after\nyou check the config and the command line option).\n\nI however fail to see why that necessitates to change use_index to\nno_index, making the code harder to follow by introducing double\nnegation.\n"},{"id":"275701","messageId":"xmqqio2zx5mz.fsf@gitster.mtv.corp.google.com","threadId":"41155","inReplyTo":"xmqqmvsbx62d.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-11T19:44:52Z","receivedAt":"2016-01-11T19:44:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I however fail to see why that necessitates to change use_index to\n> no_index, making the code harder to follow by introducing double\n> negation.\n\nOh, perhaps your thinking is that there are multiple ways that\nuse_index can become 0 (i.e. it could come from the config, could\ncome from an explicit --no-index, or it could come from the new\ndefault behaviour), and the error messages deep in the callchain\n(long after option parsing is done) want to react to these\ndifferences.\n\nTo that I am somewhat sympathetic, but then use_index can become 1\n(rather, no_index can become 0) in multiple ways (i.e. it can be\nbecause the user is just using the command as designed for its\nprimary use case, or the user explicitly said --no-no-index), so I\nam not sure.\n\nIn either case, I do not have a strong objection. Avoiding double\nnegation is merely a moderately strong general preference.\n"},{"id":"275706","messageId":"20160111210102.GE10612@hank","threadId":"41155","inReplyTo":"xmqqio2zx5mz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] builtin/grep: allow implicit --no-index","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T21:01:02Z","receivedAt":"2016-01-11T21:01:02Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/11, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n> > I however fail to see why that necessitates to change use_index to\n> > no_index, making the code harder to follow by introducing double\n> > negation.\n>\n> Oh, perhaps your thinking is that there are multiple ways that\n> use_index can become 0 (i.e. it could come from the config, could\n> come from an explicit --no-index, or it could come from the new\n> default behaviour), and the error messages deep in the callchain\n> (long after option parsing is done) want to react to these\n> differences.\n\nYes, that's what I was thinking, sorry if I wasn't clear before.\nThough I think the a bit more generic error messages are just fine, so\nwe can avoid the double negation.\n\n> To that I am somewhat sympathetic, but then use_index can become 1\n> (rather, no_index can become 0) in multiple ways (i.e. it can be\n> because the user is just using the command as designed for its\n> primary use case, or the user explicitly said --no-no-index), so I\n> am not sure.\n>\n> In either case, I do not have a strong objection. Avoiding double\n> negation is merely a moderately strong general preference.\n"},{"id":"275712","messageId":"1452547580-30687-1-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452435597-12099-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v3 0/2] grep: add fallbackToNoIndex config option","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T21:26:18Z","receivedAt":"2016-01-11T21:26:18Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Previous rounds are at $gmane/283619 and $gmane/283670. Thanks to\nJunio and Peff for the input after the last round.\n\nAfter the discussion I decided to drop the --use-index command line\nargument, and use a fallbackToNoIndex config option instead. In\naddition I dropped the second patch, keeping the use_index name for\nthe variable used to keep track which mode should be used.\n\nThomas Gummerer (2):\n t7810: correct --no-index test\n builtin/grep: add grep.fallbackToNoIndex config\n\n Documentation/config.txt | 4 ++++\n Documentation/git-grep.txt | 4 ++++\n builtin/grep.c | 23 +++++++++++++++++-----\n t/t7810-grep.sh | 49 ++++++++++++++++++++++++++++++++++++++++++----\n 4 files changed, 71 insertions(+), 9 deletions(-)\n\n-- \n2.7.0.2.gafd9bce.dirty\n"},{"id":"275713","messageId":"1452547580-30687-2-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452547580-30687-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v3 1/2] t7810: correct --no-index test","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T21:26:19Z","receivedAt":"2016-01-11T21:26:19Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"GIT_CEILING_DIRECTORIES doesn't prevent chdir up into another directory\nwhile looking for a repository directory if it is equal to the current\ndirectory. Because of this, the test which claims to test the git grep\n--no-index command outside of a repository actually tests it inside of a\nrepository. The test_must_fail assertions still pass because the git\ngrep only looks at untracked files and therefore no file matches, but\nnot because it's run outside of a repository as it was originally\nintended.\n\nSet the GIT_CEILING_DIRECTORIES environment variable to the parent\ndirectory of the directory in which the git grep command is executed, to\nmake sure it is actually run outside of a git repository.\n\nIn addition, the && chain was broken in a couple of places in the same\ntest, fix that.\n\nSigned-off-by: Thomas Gummerer \n---\n t/t7810-grep.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 028ffe4..cc4b97d 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -791,12 +791,12 @@ test_expect_success 'outside of git repository' '\n \t} >non/expect.full &&\n \techo file2:world >non/expect.sub &&\n \t(\n-\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non/git\" &&\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n \t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index o >../actual.full &&\n-\t\ttest_cmp ../expect.full ../actual.full\n+\t\ttest_cmp ../expect.full ../actual.full &&\n \t\tcd sub &&\n \t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index o >../../actual.sub &&\n@@ -805,7 +805,7 @@ test_expect_success 'outside of git repository' '\n \n \techo \".*o*\" >non/git/.gitignore &&\n \t(\n-\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non/git\" &&\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n \t\ttest_must_fail git grep o &&\n@@ -813,7 +813,7 @@ test_expect_success 'outside of git repository' '\n \t\ttest_cmp ../expect.full ../actual.full &&\n \n \t\t{\n-\t\t\techo \".gitignore:.*o*\"\n+\t\t\techo \".gitignore:.*o*\" &&\n \t\t\tcat ../expect.full\n \t\t} >../expect.with.ignored &&\n \t\tgit grep --no-index --no-exclude o >../actual.full &&\n-- \n2.7.0.2.gafd9bce.dirty\n"},{"id":"275714","messageId":"1452547580-30687-3-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452547580-30687-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v3 2/2] builtin/grep: add grep.fallbackToNoIndex config","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T21:26:20Z","receivedAt":"2016-01-11T21:26:20Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Currently when git grep is used outside of a git repository without the\n--no-index option git simply dies. For convenience, add a\ngrep.fallbackToNoIndex configuration variable. If set to true, git grep\nbehaves like git grep --no-index if it is run outside of a git\nrepository. It defaults to false, preserving the current behavior.\n\nHelped-by: Eric Sunshine \nSigned-off-by: Thomas Gummerer \n---\n Documentation/config.txt | 4 ++++\n Documentation/git-grep.txt | 4 ++++\n builtin/grep.c | 23 ++++++++++++++++++-----\n t/t7810-grep.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 67 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f617886..8d51f80 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1450,6 +1450,10 @@ grep.extendedRegexp::\n \toption is ignored when the 'grep.patternType' option is set to a value\n \tother than 'default'.\n \n+grep.fallbackToNoIndex::\n+\tIf set to true, fall back to git grep --no-index if git grep\n+\tis executed outside of a git repository. Defaults to false.\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 4a44d6d..15b9033 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -56,6 +56,10 @@ grep.extendedRegexp::\n grep.fullName::\n \tIf set to true, enable '--full-name' option by default.\n \n+grep.fallbackToNoIndex::\n+\tIf set to true, fall back to git grep --no-index if git grep\n+\tis executed outside of a git repository. Defaults to false.\n+\n \n OPTIONS\n -------\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 4229cae..5efe9bb 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -755,9 +755,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n \n-\tif (use_index && !startup_info->have_repository)\n-\t\t/* die the same way as if we did it at the beginning */\n-\t\tsetup_git_directory();\n+\tif (use_index && !startup_info->have_repository) {\n+\t\tint fallback = 0;\n+\t\tgit_config_get_bool(\"grep.fallbacktonoindex\", &fallback);\n+\t\tif (fallback) {\n+\t\t\tint nongit = 0;\n+\n+\t\t\tsetup_git_directory_gently(&nongit);\n+\t\t\tif (nongit)\n+\t\t\t\tuse_index = 0;\n+\t\t} else {\n+\t\t\t/* die the same way as if we did it at the beginning */\n+\t\t\tsetup_git_directory();\n+\t\t}\n+\t}\n \n \t/*\n \t * skip a -- separator; we know it cannot be\n@@ -874,12 +885,14 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tsetup_pager();\n \n \tif (!use_index && (untracked || cached))\n-\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n+\t\tdie(_(\"--cached or --untracked cannot be used with --no-index \"\n+\t\t \"or outside of a git repository\"));\n \n \tif (!use_index || untracked) {\n \t\tint use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude;\n \t\tif (list.nr)\n-\t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n+\t\t\tdie(_(\"git grep outside of a repository, --no-index or \"\n+\t\t\t \"--untracked cannot be used with revs.\"));\n \t\thit = grep_directory(&opt, &pathspec, use_exclude);\n \t} else if (0 <= opt_exclude) {\n \t\tdie(_(\"--[no-]exclude-standard cannot be used for tracked contents.\"));\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex cc4b97d..b540944 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -821,6 +821,47 @@ test_expect_success 'outside of git repository' '\n \t)\n '\n \n+test_expect_success 'outside of git repository with fallbackToNoIndex' '\n+\trm -fr non &&\n+\tmkdir -p non/git/sub &&\n+\techo hello >non/git/file1 &&\n+\techo world >non/git/sub/file2 &&\n+\tcat <<-\\EOF >non/expect.full &&\n+\tfile1:hello\n+\tsub/file2:world\n+\tEOF\n+\techo file2:world >non/expect.sub &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\ttest_must_fail git -c grep.fallbackToNoIndex=false grep o &&\n+\t\tgit -c grep.fallbackToNoIndex=true grep o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\t\tcd sub &&\n+\t\ttest_must_fail git -c grep.fallbackToNoIndex=false grep o &&\n+\t\tgit -c grep.fallbackToNoIndex=true grep o >../../actual.sub &&\n+\t\ttest_cmp ../../expect.sub ../../actual.sub\n+\t) &&\n+\n+\techo \".*o*\" >non/git/.gitignore &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\ttest_must_fail git -c grep.fallbackToNoIndex=false grep o &&\n+\t\tgit -c grep.fallbackToNoIndex=true grep --exclude-standard o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\n+\t\t{\n+\t\t\techo \".gitignore:.*o*\" &&\n+\t\t\tcat ../expect.full\n+\t\t} >../expect.with.ignored &&\n+\t\tgit -c grep.fallbackToNoIndex grep --no-exclude o >../actual.full &&\n+\t\ttest_cmp ../expect.with.ignored ../actual.full\n+\t)\n+'\n+\n test_expect_success 'inside git repository but with --no-index' '\n \trm -fr is &&\n \tmkdir -p is/git/sub &&\n-- \n2.7.0.2.gafd9bce.dirty\n"},{"id":"275717","messageId":"20160111214846.GC21131@sigill.intra.peff.net","threadId":"41155","inReplyTo":"1452547580-30687-3-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH v3 2/2] builtin/grep: add grep.fallbackToNoIndex config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-11T21:48:47Z","receivedAt":"2016-01-11T21:48:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 11, 2016 at 10:26:20PM +0100, Thomas Gummerer wrote:\n\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 4229cae..5efe9bb 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -755,9 +755,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n> \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n> \n> -\tif (use_index && !startup_info->have_repository)\n> -\t\t/* die the same way as if we did it at the beginning */\n> -\t\tsetup_git_directory();\n> +\tif (use_index && !startup_info->have_repository) {\n> +\t\tint fallback = 0;\n> +\t\tgit_config_get_bool(\"grep.fallbacktonoindex\", &fallback);\n> +\t\tif (fallback) {\n> +\t\t\tint nongit = 0;\n> +\n> +\t\t\tsetup_git_directory_gently(&nongit);\n> +\t\t\tif (nongit)\n> +\t\t\t\tuse_index = 0;\n> +\t\t} else {\n> +\t\t\t/* die the same way as if we did it at the beginning */\n> +\t\t\tsetup_git_directory();\n> +\t\t}\n> +\t}\n\nHmm. We used to have problems accessing config before calling\nsetup_git_directory(). I am not sure if that is still the case, though.\n\nI guess the startup sequence is muddied here, though. cmd_grep() is\nmarked as RUN_SETUP_GENTLY, so we would have already run setup, and here\nwe are following the \"we are not in a repository\" code path (i.e., we\nsaw \"!startup_info->have_repository\").\n\nAnd the existing setup_git_directory() is just there to die(), as the\ncomment explains. So what is the new setup_git_directory_gently() doing?\nWe know we've already done setup, and that we're not in a git repo,\nright? Shouldn't we just be able to set use_index to 0 and keep going?\nUnder what circumstances would it _not_ return \"nongit == 1\"?\n\n> @@ -874,12 +885,14 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \t\tsetup_pager();\n> \n> \tif (!use_index && (untracked || cached))\n> -\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n> +\t\tdie(_(\"--cached or --untracked cannot be used with --no-index \"\n> +\t\t \"or outside of a git repository\"));\n\nI'm lukewarm on this (and the other) change. What you've written is\ntechnically correct, but it's getting rather verbose. We've presented\nthe option already as \"turn on --no-index by default outside a\nrepository\", so I'm not sure we need to clarify it here. Since it's a\nfeature people must turn on manually, presumably they would know that.\n\nI don't know, maybe it would help somebody. I'm not strongly opposed.\n\n-Peff\n"},{"id":"275720","messageId":"20160111223524.GF10612@hank","threadId":"41155","inReplyTo":"20160111214846.GC21131@sigill.intra.peff.net","subject":"Re: [PATCH v3 2/2] builtin/grep: add grep.fallbackToNoIndex config","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-11T22:35:24Z","receivedAt":"2016-01-11T22:35:24Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/11, Jeff King wrote:\n> On Mon, Jan 11, 2016 at 10:26:20PM +0100, Thomas Gummerer wrote:\n>\n> > diff --git a/builtin/grep.c b/builtin/grep.c\n> > index 4229cae..5efe9bb 100644\n> > --- a/builtin/grep.c\n> > +++ b/builtin/grep.c\n> > @@ -755,9 +755,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> > \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n> > \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n> >\n> > -\tif (use_index && !startup_info->have_repository)\n> > -\t\t/* die the same way as if we did it at the beginning */\n> > -\t\tsetup_git_directory();\n> > +\tif (use_index && !startup_info->have_repository) {\n> > +\t\tint fallback = 0;\n> > +\t\tgit_config_get_bool(\"grep.fallbacktonoindex\", &fallback);\n> > +\t\tif (fallback) {\n> > +\t\t\tint nongit = 0;\n> > +\n> > +\t\t\tsetup_git_directory_gently(&nongit);\n> > +\t\t\tif (nongit)\n> > +\t\t\t\tuse_index = 0;\n> > +\t\t} else {\n> > +\t\t\t/* die the same way as if we did it at the beginning */\n> > +\t\t\tsetup_git_directory();\n> > +\t\t}\n> > +\t}\n>\n> Hmm. We used to have problems accessing config before calling\n> setup_git_directory(). I am not sure if that is still the case, though.\n\nA few lines earlier, git_config() is called, so I guess we're fine\nhere.\n\n> I guess the startup sequence is muddied here, though. cmd_grep() is\n> marked as RUN_SETUP_GENTLY, so we would have already run setup, and here\n> we are following the \"we are not in a repository\" code path (i.e., we\n> saw \"!startup_info->have_repository\").\n>\n> And the existing setup_git_directory() is just there to die(), as the\n> comment explains. So what is the new setup_git_directory_gently() doing?\n> We know we've already done setup, and that we're not in a git repo,\n> right? Shouldn't we just be able to set use_index to 0 and keep going?\n> Under what circumstances would it _not_ return \"nongit == 1\"?\n\nI don't think it ever does return nongit == 1. I'm not sure why I\nthought that could be the case, will fix in the re-roll.\n\n> > @@ -874,12 +885,14 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> > \t\tsetup_pager();\n> >\n> > \tif (!use_index && (untracked || cached))\n> > -\t\tdie(_(\"--cached or --untracked cannot be used with --no-index.\"));\n> > +\t\tdie(_(\"--cached or --untracked cannot be used with --no-index \"\n> > +\t\t \"or outside of a git repository\"));\n>\n> I'm lukewarm on this (and the other) change. What you've written is\n> technically correct, but it's getting rather verbose. We've presented\n> the option already as \"turn on --no-index by default outside a\n> repository\", so I'm not sure we need to clarify it here. Since it's a\n> feature people must turn on manually, presumably they would know that.\n\nYes, I think I agree with you. The original error message should give\nenough information for users. Will revert the change in the re-roll.\n\n> I don't know, maybe it would help somebody. I'm not strongly opposed.\n>\n> -Peff\n"},{"id":"275776","messageId":"1452595226-14616-1-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452547580-30687-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v4 0/2] grep: add fallbackToNoIndex config option","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-12T10:40:24Z","receivedAt":"2016-01-12T10:40:24Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Previous rounds are at $gmane/283619 and $gmane/283670. (Gmane seems\nto lag behind, I can't seem to find v3 there). Thanks to Peff for the\nreview of the previous round.\n\nChanges since the previous round:\n - removed unnecessary call to setup_git_directory_gently()\n - reverted the change to the error messages, as the originals should\n provide enough information to the user.\n\nThomas Gummerer (2):\n t7810: correct --no-index test\n builtin/grep: add grep.fallbackToNoIndex config\n\n Documentation/config.txt | 4 ++++\n Documentation/git-grep.txt | 4 ++++\n builtin/grep.c | 12 +++++++++---\n t/t7810-grep.sh | 49 ++++++++++++++++++++++++++++++++++++++++++----\n 4 files changed, 62 insertions(+), 7 deletions(-)\n\n-- \n2.7.0.2.gcdcca30.dirty\n"},{"id":"275778","messageId":"1452595226-14616-2-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452595226-14616-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v4 1/2] t7810: correct --no-index test","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-12T10:40:25Z","receivedAt":"2016-01-12T10:40:25Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"GIT_CEILING_DIRECTORIES doesn't prevent chdir up into another directory\nwhile looking for a repository directory if it is equal to the current\ndirectory. Because of this, the test which claims to test the git grep\n--no-index command outside of a repository actually tests it inside of a\nrepository. The test_must_fail assertions still pass because the git\ngrep only looks at untracked files and therefore no file matches, but\nnot because it's run outside of a repository as it was originally\nintended.\n\nSet the GIT_CEILING_DIRECTORIES environment variable to the parent\ndirectory of the directory in which the git grep command is executed, to\nmake sure it is actually run outside of a git repository.\n\nIn addition, the && chain was broken in a couple of places in the same\ntest, fix that.\n\nSigned-off-by: Thomas Gummerer \n---\n t/t7810-grep.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 028ffe4..cc4b97d 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -791,12 +791,12 @@ test_expect_success 'outside of git repository' '\n \t} >non/expect.full &&\n \techo file2:world >non/expect.sub &&\n \t(\n-\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non/git\" &&\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n \t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index o >../actual.full &&\n-\t\ttest_cmp ../expect.full ../actual.full\n+\t\ttest_cmp ../expect.full ../actual.full &&\n \t\tcd sub &&\n \t\ttest_must_fail git grep o &&\n \t\tgit grep --no-index o >../../actual.sub &&\n@@ -805,7 +805,7 @@ test_expect_success 'outside of git repository' '\n \n \techo \".*o*\" >non/git/.gitignore &&\n \t(\n-\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non/git\" &&\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n \t\ttest_must_fail git grep o &&\n@@ -813,7 +813,7 @@ test_expect_success 'outside of git repository' '\n \t\ttest_cmp ../expect.full ../actual.full &&\n \n \t\t{\n-\t\t\techo \".gitignore:.*o*\"\n+\t\t\techo \".gitignore:.*o*\" &&\n \t\t\tcat ../expect.full\n \t\t} >../expect.with.ignored &&\n \t\tgit grep --no-index --no-exclude o >../actual.full &&\n-- \n2.7.0.2.gcdcca30.dirty\n"},{"id":"275777","messageId":"1452595226-14616-3-git-send-email-t.gummerer@gmail.com","threadId":"41155","inReplyTo":"1452595226-14616-1-git-send-email-t.gummerer@gmail.com","subject":"[PATCH v4 2/2] builtin/grep: add grep.fallbackToNoIndex config","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-12T10:40:26Z","receivedAt":"2016-01-12T10:40:26Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"Currently when git grep is used outside of a git repository without the\n--no-index option git simply dies. For convenience, add a\ngrep.fallbackToNoIndex configuration variable. If set to true, git grep\nbehaves like git grep --no-index if it is run outside of a git\nrepository. It defaults to false, preserving the current behavior.\n\nHelped-by: Jeff King \nHelped-by: Eric Sunshine \nSigned-off-by: Thomas Gummerer \n---\n Documentation/config.txt | 4 ++++\n Documentation/git-grep.txt | 4 ++++\n builtin/grep.c | 12 +++++++++---\n t/t7810-grep.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 58 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f617886..8d51f80 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1450,6 +1450,10 @@ grep.extendedRegexp::\n \toption is ignored when the 'grep.patternType' option is set to a value\n \tother than 'default'.\n \n+grep.fallbackToNoIndex::\n+\tIf set to true, fall back to git grep --no-index if git grep\n+\tis executed outside of a git repository. Defaults to false.\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 4a44d6d..15b9033 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -56,6 +56,10 @@ grep.extendedRegexp::\n grep.fullName::\n \tIf set to true, enable '--full-name' option by default.\n \n+grep.fallbackToNoIndex::\n+\tIf set to true, fall back to git grep --no-index if git grep\n+\tis executed outside of a git repository. Defaults to false.\n+\n \n OPTIONS\n -------\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 4229cae..6b7add6 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -755,9 +755,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n \n-\tif (use_index && !startup_info->have_repository)\n-\t\t/* die the same way as if we did it at the beginning */\n-\t\tsetup_git_directory();\n+\tif (use_index && !startup_info->have_repository) {\n+\t\tint fallback = 0;\n+\t\tgit_config_get_bool(\"grep.fallbacktonoindex\", &fallback);\n+\t\tif (fallback)\n+\t\t\tuse_index = 0;\n+\t\telse\n+\t\t\t/* die the same way as if we did it at the beginning */\n+\t\t\tsetup_git_directory();\n+\t}\n \n \t/*\n \t * skip a -- separator; we know it cannot be\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex cc4b97d..b540944 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -821,6 +821,47 @@ test_expect_success 'outside of git repository' '\n \t)\n '\n \n+test_expect_success 'outside of git repository with fallbackToNoIndex' '\n+\trm -fr non &&\n+\tmkdir -p non/git/sub &&\n+\techo hello >non/git/file1 &&\n+\techo world >non/git/sub/file2 &&\n+\tcat <<-\\EOF >non/expect.full &&\n+\tfile1:hello\n+\tsub/file2:world\n+\tEOF\n+\techo file2:world >non/expect.sub &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\ttest_must_fail git -c grep.fallbackToNoIndex=false grep o &&\n+\t\tgit -c grep.fallbackToNoIndex=true grep o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\t\tcd sub &&\n+\t\ttest_must_fail git -c grep.fallbackToNoIndex=false grep o &&\n+\t\tgit -c grep.fallbackToNoIndex=true grep o >../../actual.sub &&\n+\t\ttest_cmp ../../expect.sub ../../actual.sub\n+\t) &&\n+\n+\techo \".*o*\" >non/git/.gitignore &&\n+\t(\n+\t\tGIT_CEILING_DIRECTORIES=\"$(pwd)/non\" &&\n+\t\texport GIT_CEILING_DIRECTORIES &&\n+\t\tcd non/git &&\n+\t\ttest_must_fail git -c grep.fallbackToNoIndex=false grep o &&\n+\t\tgit -c grep.fallbackToNoIndex=true grep --exclude-standard o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\n+\t\t{\n+\t\t\techo \".gitignore:.*o*\" &&\n+\t\t\tcat ../expect.full\n+\t\t} >../expect.with.ignored &&\n+\t\tgit -c grep.fallbackToNoIndex grep --no-exclude o >../actual.full &&\n+\t\ttest_cmp ../expect.with.ignored ../actual.full\n+\t)\n+'\n+\n test_expect_success 'inside git repository but with --no-index' '\n \trm -fr is &&\n \tmkdir -p is/git/sub &&\n-- \n2.7.0.2.gcdcca30.dirty\n"},{"id":"275803","messageId":"20160112121106.GA18512@sigill.intra.peff.net","threadId":"41155","inReplyTo":"1452595226-14616-3-git-send-email-t.gummerer@gmail.com","subject":"Re: [PATCH v4 2/2] builtin/grep: add grep.fallbackToNoIndex config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-12T12:11:07Z","receivedAt":"2016-01-12T12:11:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 12, 2016 at 11:40:26AM +0100, Thomas Gummerer wrote:\n\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 4229cae..6b7add6 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -755,9 +755,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n> \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n> \n> -\tif (use_index && !startup_info->have_repository)\n> -\t\t/* die the same way as if we did it at the beginning */\n> -\t\tsetup_git_directory();\n> +\tif (use_index && !startup_info->have_repository) {\n> +\t\tint fallback = 0;\n> +\t\tgit_config_get_bool(\"grep.fallbacktonoindex\", &fallback);\n> +\t\tif (fallback)\n> +\t\t\tuse_index = 0;\n> +\t\telse\n> +\t\t\t/* die the same way as if we did it at the beginning */\n> +\t\t\tsetup_git_directory();\n> +\t}\n> \n> \t/*\n> \t * skip a -- separator; we know it cannot be\n\nNice. This turned out delightfully simple.\n\n-Peff\n"},{"id":"275814","messageId":"20160112155005.GG10612@hank","threadId":"41155","inReplyTo":"20160112121106.GA18512@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/2] builtin/grep: add grep.fallbackToNoIndex config","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-01-12T15:50:05Z","receivedAt":"2016-01-12T15:50:05Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 01/12, Jeff King wrote:\n> On Tue, Jan 12, 2016 at 11:40:26AM +0100, Thomas Gummerer wrote:\n>\n> > diff --git a/builtin/grep.c b/builtin/grep.c\n> > index 4229cae..6b7add6 100644\n> > --- a/builtin/grep.c\n> > +++ b/builtin/grep.c\n> > @@ -755,9 +755,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> > \t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n> > \tgrep_commit_pattern_type(pattern_type_arg, &opt);\n> >\n> > -\tif (use_index && !startup_info->have_repository)\n> > -\t\t/* die the same way as if we did it at the beginning */\n> > -\t\tsetup_git_directory();\n> > +\tif (use_index && !startup_info->have_repository) {\n> > +\t\tint fallback = 0;\n> > +\t\tgit_config_get_bool(\"grep.fallbacktonoindex\", &fallback);\n> > +\t\tif (fallback)\n> > +\t\t\tuse_index = 0;\n> > +\t\telse\n> > +\t\t\t/* die the same way as if we did it at the beginning */\n> > +\t\t\tsetup_git_directory();\n> > +\t}\n> >\n> > \t/*\n> > \t * skip a -- separator; we know it cannot be\n>\n> Nice. This turned out delightfully simple.\n\nIndeed, much simpler than I expected. Thanks for the help getting it there.\n\n> -Peff\n"}]}