From: Junio C Hamano Date: Tue, 19 Mar 2024 14:32:41 GMT Subject: Re: [PATCH 4/5] grep: introduce new config option to include untracked files Message-ID: In-Reply-To: <923c5f84c44a195d6ba08503e28851af@manjaro.org> Dragan Simic writes: >>> + if (use_index && !cached) >>> + git_config_get_bool("grep.includeuntracked", &untracked); >> Can this ever return an error? E.g. >> [grep] includeuntracked = "not really" > > 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. If that were the case, then it is not "fine". When the user triggered an operation which *requires* us to parse and interpret the meaning of an entry in their configuration file correctly in order to carry it out, and if that entry has a value that we do not consider valid, we should notice and complain, before saying "Nah, this I do not understand, so I'll do one of the two things I would have done if the value were understandable and would not tell the user which one I did". What makes it fine int his case is that git_config_get_bool() dies when the given value is not a Boolean ;-). The returned value from the function can be used to tell if the variable does not exist and the caller should decide to stuff some default value to &untracked but in this case you do not need to.