Re: [PATCH 4/5] grep: introduce new config option to include untracked files

> Dragan Simic <dsimic@manjaro.org> 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.