Subject: Re: [PATCH v1 1/2] builtin/grep.c: add --sparse option

From: Junio C Hamano <[email-masked]>
Date: Wed, 17 Aug 2022 10:13:25 -0700

Derrick Stolee <[email-masked]> writes:

> On 8/17/2022 3:56 AM, Shaoxuan Yuan wrote:
>> Add a --sparse option to `git-grep`. This option is mainly used to:
>> 
>> If searching in the index (using --cached):
>> 
>> With --sparse, proceed the action when the current cache_entry is
>
> This phrasing is awkward. It might be better to reframe to describe the
> _why_ before the _what_

Thanks for an excellent suggestion.  As a project participant, I
could guess the motivation, but couldn't link the parts of the
proposed log message to what I thought was being said X-<.  The
below is much clearer.

>   When the '--cached' option is used with the 'git grep' command, the
>   search is limited to the blobs found in the index, not in the worktree.
>   If the user has enabled sparse-checkout, this might present more results
>   than they would like, since the files outside of the sparse-checkout are
>   unlikely to be important to them.

Great.  As an explanation of the reasoning behind the design
decision, I do not think it is bad to go even stronger than "might
... would like" and assume or declare that those users who use
sparse-checkout are the ones who do NOT want to see the parts of the
tree that are sparsed out.  And based on that assumption, "grep" and
"grep --cached" should not bother reporting hit from the part that
the user is not interested in.

By stating the design and the reasoning behind that decision clearly
like so, we allow future developers to reconsider the earlier design
decision more easily.  In 7 years, they may find that the Git users
in their era use sparse-checkout even when they still care about the
contents in the sparsed out area, in which case the basic assumption
behind this change is no longer valid and would allow them to make
"grep" and "grep --cached" behave differently.

>   Change the default behavior of 'git grep' to focus on the files within
>   the sparse-checkout definition. To enable the previous behavior, add a
>   '--sparse' option to 'git grep' that triggers the old behavior that
>   inspects paths outside of the sparse-checkout definition when paired
>   with the '--cached' option.

Yup.  Is that "--sparse" or "--unsparse"?  We are busting the sparse
boundary and looking for everything, and calling the option to do so
"--sparse" somehow feels counter-intuitive, at least to me.

Thanks.