From: Dragan Simic Date: Tue, 19 Mar 2024 05:47:48 GMT Subject: Re: [PATCH 4/5] grep: introduce new config option to include untracked files Message-ID: <923c5f84c44a195d6ba08503e28851af@manjaro.org> In-Reply-To: On 2024-03-19 01:58, Junio C Hamano wrote: > Dragan Simic writes: >> Add new configuration option grep.includeUntracked that enables >> --untracked >> option by default. This pretty much follows the logic established by >> the >> already existing configuration option grep.fallbackToNoIndex, while >> also >> respecting the dependencies of the --untracked option. >> >> Also add a few automated tests to the t7810, to cover the new >> configuration > > Do we have any non-automated tests in t7810? Good point, will be removed in the v2, if we get there. Tying "automated" to "test" is just an old habit of mine. >> option by replicating the already existing tests for --untracked. >> >> Signed-off-by: Dragan Simic >> --- >> builtin/grep.c | 3 +++ >> t/t7810-grep.sh | 9 +++++++++ >> 2 files changed, 12 insertions(+) >> >> diff --git a/builtin/grep.c b/builtin/grep.c >> index af89c8b5cb19..71d94126fb6e 100644 >> --- a/builtin/grep.c >> +++ b/builtin/grep.c >> @@ -1041,6 +1041,9 @@ int cmd_grep(int argc, const char **argv, const >> char *prefix) >> the_repository->settings.command_requires_full_index = 0; >> } >> >> + if (use_index && !cached) >> + git_config_get_bool("grep.includeuntracked", &untracked); > > Can this ever return an error? E.g. > > [grep] includeuntracked = "not really" > > How badly would setting this configuration variable break third > party tools that assume their "git grep" invocation without the > "--untracked" option would not yield hits from untracked files? After a brief inspection of the code in cache.c, git_config_get_bool() always returns either 0 or 1, so we should be fine. Thus, any strangeness in a configuration file would end up not enabling this option. As I already explained in my earlier response, [1] I think that the usability of this option outweighs the possible issues it may cause to some users. [1] https://lore.kernel.org/git/c68a6d94bb02e5d9aa2f81bee022baa8@manjaro.org/