{"thread":{"id":"61147","subject":"[PATCH 3/5] grep docs: describe --no-index further","startedAt":"2024-03-18T17:03:38Z","lastAt":"2024-03-19T17:48:58Z","messageCount":22,"participants":["Dragan Simic","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"490884","messageId":"3e6136028ca3327b027d57c46fb8155ef3c0d6c5.1710781235.git.dsimic@manjaro.org","threadId":"61147","inReplyTo":"cover.1710781235.git.dsimic@manjaro.org","subject":"[PATCH 3/5] grep docs: describe --no-index further","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-18T17:03:23Z","receivedAt":"2024-03-18T17:03:38Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Describe the dependency between --no-index and either of the --cached and\n--untracked options, which cannot be used together.\n\nAs part of that, shuffle a couple of the options, to make the documentation\nflow a bit better; it makes more sense to describe first the options that\nhave something in common, and to after that describe an option that has some\ndependency on the already described options.\n\nSigned-off-by: Dragan Simic \n---\n Documentation/git-grep.txt | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex ade69f00ebdd..b377523381bb 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -28,7 +28,7 @@ SYNOPSIS\n \t [-f ] [-e] \n \t [--and|--or|--not|(|)|-e ...]\n \t [--recurse-submodules] [--parent-basename ]\n-\t [ [--[no-]exclude-standard] [--cached | --no-index | --untracked] | ...]\n+\t [ [--[no-]exclude-standard] [--cached | --untracked | --no-index] | ...]\n \t [--] [...]\n \n DESCRIPTION\n@@ -45,13 +45,15 @@ OPTIONS\n \tInstead of searching tracked files in the working tree, search\n \tblobs registered in the index file.\n \n---no-index::\n-\tSearch files in the current directory that is not managed by Git.\n-\n --untracked::\n \tIn addition to searching in the tracked files in the working\n \ttree, search also in untracked files.\n \n+--no-index::\n+\tSearch files in the current directory that is not managed by Git.\n+\tThis option cannot be used together with `--cached` or `--untracked`.\n+\tSee also `grep.fallbackToNoIndex` in CONFIGURATION below.\n+\n --no-exclude-standard::\n \tAlso search in ignored files by not honoring the `.gitignore`\n \tmechanism. Only useful with `--untracked`.\n"},{"id":"490885","messageId":"9f70eeb4f04a874a2036e1d8c61f3b7ec130663a.1710781235.git.dsimic@manjaro.org","threadId":"61147","inReplyTo":"cover.1710781235.git.dsimic@manjaro.org","subject":"[PATCH 4/5] grep: introduce new config option to include untracked files","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-18T17:03:24Z","receivedAt":"2024-03-18T17:03:38Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Add new configuration option grep.includeUntracked that enables --untracked\noption by default. This pretty much follows the logic established by the\nalready existing configuration option grep.fallbackToNoIndex, while also\nrespecting the dependencies of the --untracked option.\n\nAlso add a few automated tests to the t7810, to cover the new configuration\noption by replicating the already existing tests for --untracked.\n\nSigned-off-by: Dragan Simic \n---\n builtin/grep.c | 3 +++\n t/t7810-grep.sh | 9 +++++++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex af89c8b5cb19..71d94126fb6e 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -1041,6 +1041,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tthe_repository->settings.command_requires_full_index = 0;\n \t}\n \n+\tif (use_index && !cached)\n+\t\tgit_config_get_bool(\"grep.includeuntracked\", &untracked);\n+\n \tif (use_index && !startup_info->have_repository) {\n \t\tint fallback = 0;\n \t\tgit_config_get_bool(\"grep.fallbacktonoindex\", &fallback);\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 875dcfd98f3a..de93936d585f 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -1298,20 +1298,29 @@ test_expect_success 'inside git repository but with --no-index' '\n \t\tgit grep --untracked o >../actual.unignored &&\n \t\ttest_cmp ../expect.unignored ../actual.unignored &&\n \n+\t\tgit -c grep.includeUntracked=true grep o >../actual.unignored &&\n+\t\ttest_cmp ../expect.unignored ../actual.unignored &&\n+\n \t\tgit grep --no-index o >../actual.full &&\n \t\ttest_cmp ../expect.full ../actual.full &&\n \n+\t\tgit -c grep.includeUntracked=true grep --no-index o >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\n \t\tgit grep --no-index --exclude-standard o >../actual.unignored &&\n \t\ttest_cmp ../expect.unignored ../actual.unignored &&\n \n \t\tcd sub &&\n \t\ttest_must_fail git grep o >../../actual.sub &&\n \t\ttest_must_be_empty ../../actual.sub &&\n \n \t\tgit grep --no-index o >../../actual.sub &&\n \t\ttest_cmp ../../expect.sub ../../actual.sub &&\n \n \t\tgit grep --untracked o >../../actual.sub &&\n+\t\ttest_cmp ../../expect.sub ../../actual.sub &&\n+\n+\t\tgit -c grep.includeUntracked=true grep o >../../actual.sub &&\n \t\ttest_cmp ../../expect.sub ../../actual.sub\n \t)\n '\n"},{"id":"490886","messageId":"784912a8d9156fa00ddee218fd600254d7bab160.1710781235.git.dsimic@manjaro.org","threadId":"61147","inReplyTo":"cover.1710781235.git.dsimic@manjaro.org","subject":"[PATCH 2/5] grep docs: describe --recurse-submodules further and improve formatting a bit","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-18T17:03:22Z","receivedAt":"2024-03-18T17:03:38Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Clarify that --recurse-submodules cannot be used together with --untracked,\nand improve the formatting in a couple of places, to make it visually clear\nthat those are the commands or the names of configuration options.\n\nSigned-off-by: Dragan Simic \n---\n Documentation/config/grep.txt | 2 +-\n Documentation/git-grep.txt | 6 +++---\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config/grep.txt b/Documentation/config/grep.txt\nindex e521f20390ce..10041f27b0c8 100644\n--- a/Documentation/config/grep.txt\n+++ b/Documentation/config/grep.txt\n@@ -24,5 +24,5 @@ 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+\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.\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 0d0103c780af..ade69f00ebdd 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -65,8 +65,8 @@ OPTIONS\n \tRecursively search in each submodule that is active and\n \tchecked out in the repository. When used in combination with the\n \t option the prefix of all submodule output will be the name of\n-\tthe parent project's object. This option has no effect\n-\tif `--no-index` is given.\n+\tthe parent project's object. This option cannot be used together\n+\twith `--untracked`, and it has no effect if `--no-index` is specified.\n \n -a::\n --text::\n@@ -178,7 +178,7 @@ providing this option will cause it to die.\n \tUse \\0 as the delimiter for pathnames in the output, and print\n \tthem verbatim. Without this option, pathnames with \"unusual\"\n \tcharacters are quoted as explained for the configuration\n-\tvariable core.quotePath (see linkgit:git-config[1]).\n+\tvariable `core.quotePath` (see linkgit:git-config[1]).\n \n -o::\n --only-matching::\n"},{"id":"490887","messageId":"96b81f3573d3f29bb97e77d623be0d53cf8085b0.1710781235.git.dsimic@manjaro.org","threadId":"61147","inReplyTo":"cover.1710781235.git.dsimic@manjaro.org","subject":"[PATCH 1/5] grep: perform some minor code and comment cleanups","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-18T17:03:21Z","receivedAt":"2024-03-18T17:03:38Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Move some variable definitions around, and reflow one comment block, to\nmake the code a bit neater after spotting those slightly unpolished areas.\nThere are no functional changes to the source code.\n\nSigned-off-by: Dragan Simic \n---\n builtin/grep.c | 21 ++++++++-------------\n 1 file changed, 8 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 982bcfc4b1df..af89c8b5cb19 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -623,13 +623,13 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,\n \t\t struct tree_desc *tree, struct strbuf *base, int tn_len,\n \t\t int check_attr)\n {\n-\tstruct repository *repo = opt->repo;\n-\tint hit = 0;\n+\tint hit = 0, name_base_len = 0;\n+\tint old_baselen = base->len;\n \tenum interesting match = entry_not_interesting;\n+\tstruct repository *repo = opt->repo;\n \tstruct name_entry entry;\n-\tint old_baselen = base->len;\n \tstruct strbuf name = STRBUF_INIT;\n-\tint name_base_len = 0;\n+\n \tif (repo->submodule_prefix) {\n \t\tstrbuf_addstr(&name, repo->submodule_prefix);\n \t\tname_base_len = name.len;\n@@ -890,19 +890,15 @@ static int pattern_callback(const struct option *opt, const char *arg,\n \n int cmd_grep(int argc, const char **argv, const char *prefix)\n {\n-\tint hit = 0;\n+\tint hit = 0, seen_dashdash = 0, use_index = 1;\n \tint cached = 0, untracked = 0, opt_exclude = -1;\n-\tint seen_dashdash = 0;\n \tint external_grep_allowed__ignored;\n+\tint i, dummy, allow_revs;\n \tconst char *show_in_pager = NULL, *default_pager = \"dummy\";\n \tstruct grep_opt opt;\n \tstruct object_array list = OBJECT_ARRAY_INIT;\n \tstruct pathspec pathspec;\n \tstruct string_list path_list = STRING_LIST_INIT_DUP;\n-\tint i;\n-\tint dummy;\n-\tint use_index = 1;\n-\tint allow_revs;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"cached\", &cached,\n@@ -1059,9 +1055,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\trecurse_submodules = 0;\n \n \t/*\n-\t * skip a -- separator; we know it cannot be\n-\t * separating revisions from pathnames if\n-\t * we haven't even had any patterns yet\n+\t * skip a -- separator; we know it cannot be separating revisions\n+\t * from pathnames if we haven't even had any patterns yet\n \t */\n \tif (argc > 0 && !opt.pattern_list && !strcmp(argv[0], \"--\")) {\n \t\targv++;\n"},{"id":"490888","messageId":"cover.1710781235.git.dsimic@manjaro.org","threadId":"61147","inReplyTo":null,"subject":"[PATCH 0/5] New config option for git-grep to include untracked files","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-18T17:03:20Z","receivedAt":"2024-03-18T17:03:38Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"This patch series introduces new config option grep.includeUntracked,\nwhich makes the untracked files also searched by default when git-grep(1)\nis invoked, in addition to searching the tracked files. This is quite\nhandy when someone expects git-grep(1) to mimic grep(1) even better, when\nit comes to the selection of searched files.\n\nSetting grep.includeUntracked to true in one's git configuration should\nhave no ill effects to various scripts, which presumably shouldn't rely\non expensive operations such as git-grep(1).\n\nThis series also performs some related cleanups and small improvements,\nwhich are extracted into separate patches.\n\nDragan Simic (5):\n grep: perform some minor code and comment cleanups\n grep docs: describe --recurse-submodules further and improve\n formatting a bit\n grep docs: describe --no-index further\n grep: introduce new config option to include untracked files\n grep docs: describe new config option to include untracked files\n\n Documentation/config/grep.txt | 8 +++++++-\n Documentation/git-grep.txt | 19 +++++++++++--------\n builtin/grep.c | 24 +++++++++++-------------\n t/t7810-grep.sh | 9 +++++++++\n 4 files changed, 38 insertions(+), 22 deletions(-)\n\n"},{"id":"490889","messageId":"9ce006509306136bb2354006e3b14ceaf26d0d32.1710781235.git.dsimic@manjaro.org","threadId":"61147","inReplyTo":"cover.1710781235.git.dsimic@manjaro.org","subject":"[PATCH 5/5] grep docs: describe new config option to include untracked files","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-18T17:03:25Z","receivedAt":"2024-03-18T17:03:43Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Describe the new configuration option grep.includeUntracked, including the\ndependencies with the already existing options, and the conditions that\nmake this option ignored, which allows other options to be used while the\nnew option is enabled in one's git configuration file.\n\nSigned-off-by: Dragan Simic \n---\n Documentation/config/grep.txt | 6 ++++++\n Documentation/git-grep.txt | 3 ++-\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/grep.txt b/Documentation/config/grep.txt\nindex 10041f27b0c8..32f086997596 100644\n--- a/Documentation/config/grep.txt\n+++ b/Documentation/config/grep.txt\n@@ -23,6 +23,12 @@ grep.threads::\n grep.fullName::\n \tIf set to true, enable `--full-name` option by default.\n \n+grep.includeUntracked::\n+\tIf set to true, enable `--untracked` option by default, to search also\n+\tin untracked files, in addition to searching in the tracked files in the\n+\tworking tree. If `--cached` or `--no-index` is specified, this option is\n+\tignored. Also, it cannot be enabled together with `submodule.recurse`.\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.\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex b377523381bb..af5f6572df16 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -47,7 +47,8 @@ OPTIONS\n \n --untracked::\n \tIn addition to searching in the tracked files in the working\n-\ttree, search also in untracked files.\n+\ttree, search also in untracked files. See also `grep.includeUntracked`\n+\tin CONFIGURATION below.\n \n --no-index::\n \tSearch files in the current directory that is not managed by Git.\n"},{"id":"490896","messageId":"CAPig+cQ6Y2oOaPkKFsD41beXLHjhD++nmf59xrcswpb6_Q-sdA@mail.gmail.com","threadId":"61147","inReplyTo":"96b81f3573d3f29bb97e77d623be0d53cf8085b0.1710781235.git.dsimic@manjaro.org","subject":"Re: [PATCH 1/5] grep: perform some minor code and comment cleanups","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-03-18T19:59:41Z","receivedAt":"2024-03-18T19:59:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 18, 2024 at 1:04 PM Dragan Simic wrote:\n> Move some variable definitions around, and reflow one comment block, to\n> make the code a bit neater after spotting those slightly unpolished areas.\n> There are no functional changes to the source code.\n>\n> Signed-off-by: Dragan Simic \n> ---\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> @@ -623,13 +623,13 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,\n> - struct repository *repo = opt->repo;\n> - int hit = 0;\n> + int hit = 0, name_base_len = 0;\n> + int old_baselen = base->len;\n> enum interesting match = entry_not_interesting;\n> + struct repository *repo = opt->repo;\n> struct name_entry entry;\n> - int old_baselen = base->len;\n> struct strbuf name = STRBUF_INIT;\n> - int name_base_len = 0;\n> @@ -890,19 +890,15 @@ static int pattern_callback(const struct option *opt, const char *arg,\n> - int hit = 0;\n> + int hit = 0, seen_dashdash = 0, use_index = 1;\n> int cached = 0, untracked = 0, opt_exclude = -1;\n> - int seen_dashdash = 0;\n> int external_grep_allowed__ignored;\n> + int i, dummy, allow_revs;\n> const char *show_in_pager = NULL, *default_pager = \"dummy\";\n> struct grep_opt opt;\n> struct object_array list = OBJECT_ARRAY_INIT;\n> struct pathspec pathspec;\n> struct string_list path_list = STRING_LIST_INIT_DUP;\n> - int i;\n> - int dummy;\n> - int use_index = 1;\n> - int allow_revs;\n\nIt's entirely subjective, of course, so no right-or-wrong answer, but\nI personally do not find that this change improves code quality or\nreadability.\n\nWith my reviewer hat on, I spent an inordinate amount of time staring\nat this change trying to locate each variable's new location to verify\nthat no initializers were dropped and that the declared type hadn't\nchanged. Taking into consideration that reviewers are a limited\nresource on this project, I'd probably have skipped this patch\naltogether if I were doing this series unless these changes concretely\nhelp a subsequent patch.\n"},{"id":"490897","messageId":"CAPig+cQc8W4JOpB+TMP=czketU1U7wcY_x9bsP5T=3-XjGLhRQ@mail.gmail.com","threadId":"61147","inReplyTo":"784912a8d9156fa00ddee218fd600254d7bab160.1710781235.git.dsimic@manjaro.org","subject":"Re: [PATCH 2/5] grep docs: describe --recurse-submodules further and improve formatting a bit","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-03-18T20:02:05Z","receivedAt":"2024-03-18T20:02:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 18, 2024 at 1:04 PM Dragan Simic wrote:\n> Clarify that --recurse-submodules cannot be used together with --untracked,\n> and improve the formatting in a couple of places, to make it visually clear\n> that those are the commands or the names of configuration options.\n>\n> Signed-off-by: Dragan Simic \n> ---\n> diff --git a/Documentation/config/grep.txt b/Documentation/config/grep.txt\n> @@ -24,5 +24,5 @@ grep.fullName::\n> grep.fallbackToNoIndex::\n> - If set to true, fall back to git grep --no-index if git grep\n> + If set to true, fall back to `git grep --no-index` if `git grep`\n\nGood.\n\n> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\n> @@ -65,8 +65,8 @@ OPTIONS\n> option the prefix of all submodule output will be the name of\n> - the parent project's object. This option has no effect\n> - if `--no-index` is given.\n> + the parent project's object. This option cannot be used together\n> + with `--untracked`, and it has no effect if `--no-index` is specified.\n\nI believe that there is a patch series currently in-flight which is\nre-styling in-prose placeholders as __, so you may want to\nmake that change as well while you're touching this.\n"},{"id":"490903","messageId":"9642c7e2adc888d960ded71d79af882b@manjaro.org","threadId":"61147","inReplyTo":"CAPig+cQ6Y2oOaPkKFsD41beXLHjhD++nmf59xrcswpb6_Q-sdA@mail.gmail.com","subject":"Re: [PATCH 1/5] grep: perform some minor code and comment cleanups","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-18T22:03:17Z","receivedAt":"2024-03-18T22:03:19Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-03-18 20:59, Eric Sunshine wrote:\n> On Mon, Mar 18, 2024 at 1:04 PM Dragan Simic \n> wrote:\n>> Move some variable definitions around, and reflow one comment block, \n>> to\n>> make the code a bit neater after spotting those slightly unpolished \n>> areas.\n>> There are no functional changes to the source code.\n>> \n>> Signed-off-by: Dragan Simic \n>> ---\n>> diff --git a/builtin/grep.c b/builtin/grep.c\n>> @@ -623,13 +623,13 @@ static int grep_tree(struct grep_opt *opt, const \n>> struct pathspec *pathspec,\n>> - struct repository *repo = opt->repo;\n>> - int hit = 0;\n>> + int hit = 0, name_base_len = 0;\n>> + int old_baselen = base->len;\n>> enum interesting match = entry_not_interesting;\n>> + struct repository *repo = opt->repo;\n>> struct name_entry entry;\n>> - int old_baselen = base->len;\n>> struct strbuf name = STRBUF_INIT;\n>> - int name_base_len = 0;\n>> @@ -890,19 +890,15 @@ static int pattern_callback(const struct option \n>> *opt, const char *arg,\n>> - int hit = 0;\n>> + int hit = 0, seen_dashdash = 0, use_index = 1;\n>> int cached = 0, untracked = 0, opt_exclude = -1;\n>> - int seen_dashdash = 0;\n>> int external_grep_allowed__ignored;\n>> + int i, dummy, allow_revs;\n>> const char *show_in_pager = NULL, *default_pager = \"dummy\";\n>> struct grep_opt opt;\n>> struct object_array list = OBJECT_ARRAY_INIT;\n>> struct pathspec pathspec;\n>> struct string_list path_list = STRING_LIST_INIT_DUP;\n>> - int i;\n>> - int dummy;\n>> - int use_index = 1;\n>> - int allow_revs;\n> \n> It's entirely subjective, of course, so no right-or-wrong answer, but\n> I personally do not find that this change improves code quality or\n> readability.\n> \n> With my reviewer hat on, I spent an inordinate amount of time staring\n> at this change trying to locate each variable's new location to verify\n> that no initializers were dropped and that the declared type hadn't\n> changed. Taking into consideration that reviewers are a limited\n> resource on this project, I'd probably have skipped this patch\n> altogether if I were doing this series unless these changes concretely\n> help a subsequent patch.\n\nOh, I'm fully aware that the reviewers are a limited resource, and I do\nagree that all this is subjective. Though, I believe it makes the code\nlook nicer, which is the only reason why I performed and submitted those\nchanges in the first place.\n\nThough, maybe it would've been better if I submitted these changes as\na separate patch, instead as part of this series.\n"},{"id":"490904","messageId":"598b8683a06276f5b02dfd045c60d029@manjaro.org","threadId":"61147","inReplyTo":"CAPig+cQc8W4JOpB+TMP=czketU1U7wcY_x9bsP5T=3-XjGLhRQ@mail.gmail.com","subject":"Re: [PATCH 2/5] grep docs: describe --recurse-submodules further and improve formatting a bit","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-18T22:14:44Z","receivedAt":"2024-03-18T22:14:45Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-03-18 21:02, Eric Sunshine wrote:\n> On Mon, Mar 18, 2024 at 1:04 PM Dragan Simic \n> wrote:\n>> Clarify that --recurse-submodules cannot be used together with \n>> --untracked,\n>> and improve the formatting in a couple of places, to make it visually \n>> clear\n>> that those are the commands or the names of configuration options.\n>> \n>> Signed-off-by: Dragan Simic \n>> ---\n>> diff --git a/Documentation/config/grep.txt \n>> b/Documentation/config/grep.txt\n>> @@ -24,5 +24,5 @@ grep.fullName::\n>> grep.fallbackToNoIndex::\n>> - If set to true, fall back to git grep --no-index if git grep\n>> + If set to true, fall back to `git grep --no-index` if `git \n>> grep`\n> \n> Good.\n> \n>> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\n>> @@ -65,8 +65,8 @@ OPTIONS\n>> option the prefix of all submodule output will be the \n>> name of\n>> - the parent project's object. This option has no effect\n>> - if `--no-index` is given.\n>> + the parent project's object. This option cannot be \n>> used together\n>> + with `--untracked`, and it has no effect if `--no-index` is \n>> specified.\n> \n> I believe that there is a patch series currently in-flight which is\n> re-styling in-prose placeholders as __, so you may want to\n> make that change as well while you're touching this.\n\nThanks for the notice. I'll add that to the v2 of this series, if\nthere will be other reasons for the v2.\n"},{"id":"490911","messageId":"xmqqwmpzt6jk.fsf@gitster.g","threadId":"61147","inReplyTo":"cover.1710781235.git.dsimic@manjaro.org","subject":"Re: [PATCH 0/5] New config option for git-grep to include untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-19T00:21:51Z","receivedAt":"2024-03-19T00:21:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dragan Simic writes:\n\n> This patch series introduces new config option grep.includeUntracked,\n> which makes the untracked files also searched by default when git-grep(1)\n> is invoked, in addition to searching the tracked files.\n\nHmph. I am moderately negative on any configuration that screws\nwith the default haystack from which needle is sought for.\n\nI may often do \"git grep --cached\" but that does not mean I would\nwelcome an configuration option to make \"git grep\" search in the\nindex even when the request by the user does not have \"--cached\".\n\nInclusion of untracked sources in a sense is even worse, especially\nwhen an unsuspecting \"git grep\" user (or a script) fully expects\nthat any paths found in the output are to be found in \"git ls-files\n-s\" output but when you stray into a repository with the\nconfiguration set, that expectation suddelnly gets broken.\n\nSo, I dunno.\n\n"},{"id":"490912","messageId":"xmqqjzlzt61d.fsf@gitster.g","threadId":"61147","inReplyTo":"CAPig+cQ6Y2oOaPkKFsD41beXLHjhD++nmf59xrcswpb6_Q-sdA@mail.gmail.com","subject":"Re: [PATCH 1/5] grep: perform some minor code and comment cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-19T00:32:46Z","receivedAt":"2024-03-19T00:32:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> It's entirely subjective, of course, so no right-or-wrong answer, but\n> I personally do not find that this change improves code quality or\n> readability.\n\nI agree that this is entirely subjective. To those who wrote these\nvariable decls and inits, what they wrote was the most readable,\nwasn't it? It probably falls into the \"to some readers the existing\ncode may not be perfect, but once it is written, it is not worth a\npatch noise to fix it\" category.\n\n> With my reviewer hat on, I spent an inordinate amount of time staring\n> at this change trying to locate each variable's new location to verify\n> that no initializers were dropped and that the declared type hadn't\n> changed.\n\nIt is true that \"cleaning up, no behaviour changes intended\" patches\nare unpleasant to review. They are boring to read, and the risk of\nbreakage due to mistake is unnecessary and severe.\n\nBut if the result is objectively better, such a one-time cost may be\nworth it. We are investing into the better future. For example, we\nmay have an unsorted mess of an enum definition, and we do\nappreciate in the longer run, such a definition were \"more or less\"\nsorted within the constraint of some other criteria (like, \"errors\nget negative value\"). If the enum is a huge one, it may need some\ncareful reviewing to verify such a change that turns the unsorted\nmess into a sorted nice list, but the cost of doing so may be\njustified.\n\nDoes the change in this patch qualify as \"objectively better\"? I\ndunno.\n\nThanks.\n"},{"id":"490913","messageId":"xmqqwmpzrqfv.fsf@gitster.g","threadId":"61147","inReplyTo":"3e6136028ca3327b027d57c46fb8155ef3c0d6c5.1710781235.git.dsimic@manjaro.org","subject":"Re: [PATCH 3/5] grep docs: describe --no-index further","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-19T00:55:00Z","receivedAt":"2024-03-19T00:55:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dragan Simic writes:\n\n> ---no-index::\n> -\tSearch files in the current directory that is not managed by Git.\n> -\n> --untracked::\n> \tIn addition to searching in the tracked files in the working\n> \ttree, search also in untracked files.\n> \n> +--no-index::\n> +\tSearch files in the current directory that is not managed by Git.\n> +\tThis option cannot be used together with `--cached` or `--untracked`.\n> +\tSee also `grep.fallbackToNoIndex` in CONFIGURATION below.\n\nHmph, this is not the fault of this patch, but the description is\niffy. You can run \"git grep --no-index\" inside a directory that is\nmanaged by Git, and it behaves as if you gave --untracked, if I am\nnot mistaken.\n\nWhat \"--no-index\" does is to pretend that there is no system called\nGit and work as if it were a strange implementation of \"grep -r\".\nThe reader should be taught to understand the mode as such, because\nthat understanding will apply whether the current directory happens\nto be part of a working tree managed by git, or not under control by\ngit repository anywhere.\n\nThere is no tracked or untracked or managed or anything like that,\nas we are pretending that there is no git, so it falls naturally\nthat --cached or --untracked would not work.\n\nAnd from that point of view, swapping the order of \"--no-index\" and\n\"--untracked\" in this patch does make sense. All other options that\nspecify which haystack to find the needle in are all about git;\n\"--no-index\" truly is an oddball that pretends that we live in a\nworld without git, and as an oddball, we should move the description\nout from where it does not belong. It might also make sense to\nrethink where `--recurse-submodules` sits in the list of options\nwhile at it, as it also is an option that affects which haystack the\nsearch for the needle is carried out.\n\n> --no-exclude-standard::\n> \tAlso search in ignored files by not honoring the `.gitignore`\n> \tmechanism. Only useful with `--untracked`.\n"},{"id":"490914","messageId":"xmqqr0g7rqag.fsf@gitster.g","threadId":"61147","inReplyTo":"9f70eeb4f04a874a2036e1d8c61f3b7ec130663a.1710781235.git.dsimic@manjaro.org","subject":"Re: [PATCH 4/5] grep: introduce new config option to include untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-19T00:58:15Z","receivedAt":"2024-03-19T00:58:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dragan Simic writes:\n\n> Add new configuration option grep.includeUntracked that enables --untracked\n> option by default. This pretty much follows the logic established by the\n> already existing configuration option grep.fallbackToNoIndex, while also\n> respecting the dependencies of the --untracked option.\n>\n> Also add a few automated tests to the t7810, to cover the new configuration\n\nDo we have any non-automated tests in t7810?\n\n> option by replicating the already existing tests for --untracked.\n>\n> Signed-off-by: Dragan Simic \n> ---\n> builtin/grep.c | 3 +++\n> t/t7810-grep.sh | 9 +++++++++\n> 2 files changed, 12 insertions(+)\n>\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index af89c8b5cb19..71d94126fb6e 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -1041,6 +1041,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \t\tthe_repository->settings.command_requires_full_index = 0;\n> \t}\n> \n> +\tif (use_index && !cached)\n> +\t\tgit_config_get_bool(\"grep.includeuntracked\", &untracked);\n\nCan this ever return an error? E.g.\n\n\t[grep] includeuntracked = \"not really\"\n\nHow badly would setting this configuration variable break third\nparty tools that assume their \"git grep\" invocation without the\n\"--untracked\" option would not yield hits from untracked files?\n"},{"id":"490915","messageId":"c68a6d94bb02e5d9aa2f81bee022baa8@manjaro.org","threadId":"61147","inReplyTo":"xmqqwmpzt6jk.fsf@gitster.g","subject":"Re: [PATCH 0/5] New config option for git-grep to include untracked files","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-19T05:09:34Z","receivedAt":"2024-03-19T05:09:36Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Junio,\n\nOn 2024-03-19 01:21, Junio C Hamano wrote:\n> Dragan Simic writes:\n> \n>> This patch series introduces new config option grep.includeUntracked,\n>> which makes the untracked files also searched by default when \n>> git-grep(1)\n>> is invoked, in addition to searching the tracked files.\n> \n> Hmph. I am moderately negative on any configuration that screws\n> with the default haystack from which needle is sought for.\n> \n> I may often do \"git grep --cached\" but that does not mean I would\n> welcome an configuration option to make \"git grep\" search in the\n> index even when the request by the user does not have \"--cached\".\n> \n> Inclusion of untracked sources in a sense is even worse, especially\n> when an unsuspecting \"git grep\" user (or a script) fully expects\n> that any paths found in the output are to be found in \"git ls-files\n> -s\" output but when you stray into a repository with the\n> configuration set, that expectation suddelnly gets broken.\n\nHmm... Those are valid concerns. For example, I'd also be against\nanother configuration option that would make it possible to enable\n--cached by default, for example, because it could easily mess the\nthings up really bad.\n\nHowever, I think that the usability of this new configuration option\noutweighs the possible issues it could cause to some users. For\nexample, I quite often find myself in need to specify --untracked\nwhile running git-grep(1), to see what's found in the code changes\nI haven't staged yet, so I find this new configuration very useful.\n\nOf course, I already have a few aliases defined for grep operations,\nbut defining another one for \"grep --untracked\" simply doesn't fit\ninto my alias scheme. Obviously, that's why I implemented this new\noption, :) hoping that it would be usable to other people, too.\n\nI'm not sure how many users could be affected by the possible negative\neffects of this configuration option, by using a specific mix of git\noperations, but I think it would be quite useful.\n"},{"id":"490916","messageId":"24093dca675c49cfde39f6d6efca2342@manjaro.org","threadId":"61147","inReplyTo":"xmqqjzlzt61d.fsf@gitster.g","subject":"Re: [PATCH 1/5] grep: perform some minor code and comment cleanups","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-19T05:33:25Z","receivedAt":"2024-03-19T05:33:27Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-03-19 01:32, Junio C Hamano wrote:\n> Eric Sunshine writes:\n> \n>> It's entirely subjective, of course, so no right-or-wrong answer, but\n>> I personally do not find that this change improves code quality or\n>> readability.\n> \n> I agree that this is entirely subjective. To those who wrote these\n> variable decls and inits, what they wrote was the most readable,\n> wasn't it? It probably falls into the \"to some readers the existing\n> code may not be perfect, but once it is written, it is not worth a\n> patch noise to fix it\" category.\n\nThere's no doubt that it was the most readable form to the people who\nwrote the code, which was some time ago, but the time inevitably passes\nand the surrounding code changes over time, maybe even some new coding\nguidelines become introduced, etc.\n\nThings inevitably change, that's all I'm trying to say.\n\n>> With my reviewer hat on, I spent an inordinate amount of time staring\n>> at this change trying to locate each variable's new location to verify\n>> that no initializers were dropped and that the declared type hadn't\n>> changed.\n> \n> It is true that \"cleaning up, no behaviour changes intended\" patches\n> are unpleasant to review. They are boring to read, and the risk of\n> breakage due to mistake is unnecessary and severe.\n> \n> But if the result is objectively better, such a one-time cost may be\n> worth it. We are investing into the better future. For example, we\n> may have an unsorted mess of an enum definition, and we do\n> appreciate in the longer run, such a definition were \"more or less\"\n> sorted within the constraint of some other criteria (like, \"errors\n> get negative value\"). If the enum is a huge one, it may need some\n> careful reviewing to verify such a change that turns the unsorted\n> mess into a sorted nice list, but the cost of doing so may be\n> justified.\n\nI fully agree that reviewing code-cleanup patches it usually boring\nand often taxing. I mean, why change something that has served us\nwell for years, just to make it look nicer in someone's eyes? What\ndoes even \"nicer\" mean to everyone?\n\nWell, I sometimes look at the code as if it were a beautiful painting.\nTo some people, it doesn't matter if a painting has some rough areas,\nas long as it can be hung on a wall. To them, it's just a square thing.\nThough, to some other people, spotting such rough areas is what makes\nthe painting less beautiful to them. Paintings usually cannot be fixed\neasily, but fixing the code is much easier.\n\nOf course, \"nicer\" is very hard to define, but I believe that, in the\ndomain of computing, it could be partially defined as \"more consistent\nand flowing better\". That's what I tried to achieve with this patch,\nand I hope I've managed to convey my viewpoint.\n\n> Does the change in this patch qualify as \"objectively better\"? I\n> dunno.\n\nI'd say that these changes qualify as \"semi-objectively nicer\", but\nsurely not as \"objectively better\". For any changes to be objectively\nbetter, they'd need to introduce some functional changes to the code,\nwhile a well-performed code cleanup actually introduces no functional\nchanges.\n"},{"id":"490917","messageId":"c04a2f15bb719126da1b3dec3d9bbefb@manjaro.org","threadId":"61147","inReplyTo":"xmqqwmpzrqfv.fsf@gitster.g","subject":"Re: [PATCH 3/5] grep docs: describe --no-index further","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-19T05:37:33Z","receivedAt":"2024-03-19T05:37:34Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-03-19 01:55, Junio C Hamano wrote:\n> Dragan Simic writes:\n> \n>> ---no-index::\n>> -\tSearch files in the current directory that is not managed by Git.\n>> -\n>> --untracked::\n>> \tIn addition to searching in the tracked files in the working\n>> \ttree, search also in untracked files.\n>> \n>> +--no-index::\n>> +\tSearch files in the current directory that is not managed by Git.\n>> +\tThis option cannot be used together with `--cached` or \n>> `--untracked`.\n>> +\tSee also `grep.fallbackToNoIndex` in CONFIGURATION below.\n> \n> Hmph, this is not the fault of this patch, but the description is\n> iffy. You can run \"git grep --no-index\" inside a directory that is\n> managed by Git, and it behaves as if you gave --untracked, if I am\n> not mistaken.\n> \n> What \"--no-index\" does is to pretend that there is no system called\n> Git and work as if it were a strange implementation of \"grep -r\".\n> The reader should be taught to understand the mode as such, because\n> that understanding will apply whether the current directory happens\n> to be part of a working tree managed by git, or not under control by\n> git repository anywhere.\n> \n> There is no tracked or untracked or managed or anything like that,\n> as we are pretending that there is no git, so it falls naturally\n> that --cached or --untracked would not work.\n> \n> And from that point of view, swapping the order of \"--no-index\" and\n> \"--untracked\" in this patch does make sense. All other options that\n> specify which haystack to find the needle in are all about git;\n> \"--no-index\" truly is an oddball that pretends that we live in a\n> world without git, and as an oddball, we should move the description\n> out from where it does not belong. It might also make sense to\n> rethink where `--recurse-submodules` sits in the list of options\n> while at it, as it also is an option that affects which haystack the\n> search for the needle is carried out.\n\nThank you very much for your detailed review, as always. I'll try to\nincorporate all your remarks into the next version of this patch.\n"},{"id":"490918","messageId":"923c5f84c44a195d6ba08503e28851af@manjaro.org","threadId":"61147","inReplyTo":"xmqqr0g7rqag.fsf@gitster.g","subject":"Re: [PATCH 4/5] grep: introduce new config option to include untracked files","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-19T05:47:48Z","receivedAt":"2024-03-19T05:47:50Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-03-19 01:58, Junio C Hamano wrote:\n> Dragan Simic writes:\n>> Add new configuration option grep.includeUntracked that enables \n>> --untracked\n>> option by default. This pretty much follows the logic established by \n>> the\n>> already existing configuration option grep.fallbackToNoIndex, while \n>> also\n>> respecting the dependencies of the --untracked option.\n>> \n>> Also add a few automated tests to the t7810, to cover the new \n>> configuration\n> \n> Do we have any non-automated tests in t7810?\n\nGood point, will be removed in the v2, if we get there. Tying \n\"automated\"\nto \"test\" is just an old habit of mine.\n\n>> option by replicating the already existing tests for --untracked.\n>> \n>> Signed-off-by: Dragan Simic \n>> ---\n>> builtin/grep.c | 3 +++\n>> t/t7810-grep.sh | 9 +++++++++\n>> 2 files changed, 12 insertions(+)\n>> \n>> diff --git a/builtin/grep.c b/builtin/grep.c\n>> index af89c8b5cb19..71d94126fb6e 100644\n>> --- a/builtin/grep.c\n>> +++ b/builtin/grep.c\n>> @@ -1041,6 +1041,9 @@ int cmd_grep(int argc, const char **argv, const \n>> char *prefix)\n>> \t\tthe_repository->settings.command_requires_full_index = 0;\n>> \t}\n>> \n>> +\tif (use_index && !cached)\n>> +\t\tgit_config_get_bool(\"grep.includeuntracked\", &untracked);\n> \n> Can this ever return an error? E.g.\n> \n> \t[grep] includeuntracked = \"not really\"\n> \n> How badly would setting this configuration variable break third\n> party tools that assume their \"git grep\" invocation without the\n> \"--untracked\" option would not yield hits from untracked files?\n\nAfter a brief inspection of the code in cache.c, git_config_get_bool()\nalways returns either 0 or 1, so we should be fine. Thus, any\nstrangeness in a configuration file would end up not enabling\nthis option.\n\nAs I already explained in my earlier response, [1] I think that\nthe usability of this option outweighs the possible issues it may\ncause to some users.\n\n[1] \nhttps://lore.kernel.org/git/c68a6d94bb02e5d9aa2f81bee022baa8@manjaro.org/\n"},{"id":"490931","messageId":"xmqqil1is35i.fsf@gitster.g","threadId":"61147","inReplyTo":"923c5f84c44a195d6ba08503e28851af@manjaro.org","subject":"Re: [PATCH 4/5] grep: introduce new config option to include untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-19T14:32:41Z","receivedAt":"2024-03-19T14:32:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dragan Simic writes:\n\n>>> +\tif (use_index && !cached)\n>>> +\t\tgit_config_get_bool(\"grep.includeuntracked\", &untracked);\n>> Can this ever return an error? E.g.\n>> \t[grep] includeuntracked = \"not really\"\n>\n> After a brief inspection of the code in cache.c, git_config_get_bool()\n> always returns either 0 or 1, so we should be fine. Thus, any\n> strangeness in a configuration file would end up not enabling\n> this option.\n\nIf that were the case, then it is not \"fine\".\n\nWhen the user triggered an operation which *requires* us to parse\nand interpret the meaning of an entry in their configuration file\ncorrectly in order to carry it out, and if that entry has a value\nthat we do not consider valid, we should notice and complain, before\nsaying \"Nah, this I do not understand, so I'll do one of the two\nthings I would have done if the value were understandable and would\nnot tell the user which one I did\".\n\nWhat makes it fine int his case is that git_config_get_bool() dies\nwhen the given value is not a Boolean ;-). The returned value from\nthe function can be used to tell if the variable does not exist and\nthe caller should decide to stuff some default value to &untracked\nbut in this case you do not need to.\n"},{"id":"490932","messageId":"a00f28ff137fa1d82d00151877f036fe@manjaro.org","threadId":"61147","inReplyTo":"xmqqil1is35i.fsf@gitster.g","subject":"Re: [PATCH 4/5] grep: introduce new config option to include untracked files","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-19T14:52:28Z","receivedAt":"2024-03-19T14:52:36Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-03-19 15:32, Junio C Hamano wrote:\n> Dragan Simic writes:\n> \n>>>> +\tif (use_index && !cached)\n>>>> +\t\tgit_config_get_bool(\"grep.includeuntracked\", &untracked);\n>>> Can this ever return an error? E.g.\n>>> \t[grep] includeuntracked = \"not really\"\n>> \n>> After a brief inspection of the code in cache.c, git_config_get_bool()\n>> always returns either 0 or 1, so we should be fine. Thus, any\n>> strangeness in a configuration file would end up not enabling\n>> this option.\n> \n> If that were the case, then it is not \"fine\".\n> \n> When the user triggered an operation which *requires* us to parse\n> and interpret the meaning of an entry in their configuration file\n> correctly in order to carry it out, and if that entry has a value\n> that we do not consider valid, we should notice and complain, before\n> saying \"Nah, this I do not understand, so I'll do one of the two\n> things I would have done if the value were understandable and would\n> not tell the user which one I did\".\n> \n> What makes it fine int his case is that git_config_get_bool() dies\n> when the given value is not a Boolean ;-). The returned value from\n> the function can be used to tell if the variable does not exist and\n> the caller should decide to stuff some default value to &untracked\n> but in this case you do not need to.\n\nI'm sorry for not being meticulous enough in my previous response.\nWhat made me not pay enough attention is that it should all be already\ncovered properly with the already existing mechanisms for parsing the\ngit configuration files.\n\nIn other words, if invoking git_config_get_bool() over a user's\ngarbled git configuration file could cause any issues, that would've\nbeen another, pretty much unrelated bug.\n"},{"id":"490938","messageId":"xmqqcyrqqfrv.fsf@gitster.g","threadId":"61147","inReplyTo":"c68a6d94bb02e5d9aa2f81bee022baa8@manjaro.org","subject":"Re: [PATCH 0/5] New config option for git-grep to include untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-19T17:43:00Z","receivedAt":"2024-03-19T17:43:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dragan Simic writes:\n\n> However, I think that the usability of this new configuration option\n> outweighs the possible issues it could cause to some users.\n\n\"Screw others who share my preference and want to set this variable,\nbut use third-party tools and scripts that will get broken with this\nchange, because I do not use them\" is what we try not to say around\nhere.\n\n> For example, I quite often find myself in need to specify\n> --untracked while running git-grep(1), to see what's found in the\n> code changes I haven't staged yet, so I find this new\n> configuration very useful.\n\nThe feature (not the command line option, but the behaviour that is\ntriggered by it) surely may be useful. Otherwise we wouldn't have\nadded it. As I already said, I often find myself doing \"--cached\".\nBut that does not make it a good idea to invent a new configuration\nvariable and set \"grep.source=cached\".\n"},{"id":"490939","messageId":"d8475579f014a90b27efaf6207bc6fb0@manjaro.org","threadId":"61147","inReplyTo":"xmqqcyrqqfrv.fsf@gitster.g","subject":"Re: [PATCH 0/5] New config option for git-grep to include untracked files","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-03-19T17:48:51Z","receivedAt":"2024-03-19T17:48:58Z","isPatch":true,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2024-03-19 18:43, Junio C Hamano wrote:\n> Dragan Simic writes:\n> \n>> However, I think that the usability of this new configuration option\n>> outweighs the possible issues it could cause to some users.\n> \n> \"Screw others who share my preference and want to set this variable,\n> but use third-party tools and scripts that will get broken with this\n> change, because I do not use them\" is what we try not to say around\n> here.\n\nWell, I didn't mean it that way, but I get your point. Basically,\nthis series cannot be merged, so I'll extract the cleanup patches\nfrom it, improve them according to the earlier discussions, and\nsubmit them separately.\n\n>> For example, I quite often find myself in need to specify\n>> --untracked while running git-grep(1), to see what's found in the\n>> code changes I haven't staged yet, so I find this new\n>> configuration very useful.\n> \n> The feature (not the command line option, but the behaviour that is\n> triggered by it) surely may be useful. Otherwise we wouldn't have\n> added it. As I already said, I often find myself doing \"--cached\".\n> But that does not make it a good idea to invent a new configuration\n> variable and set \"grep.source=cached\".\n\nI see.\n"}]}