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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 17, 2022, 17:13 UTC
Message-ID: <xmqqh72ayeru.fsf@gitster.g>
In-Reply-To: <80f24382-1188-d450-d1e2-42f68c08e60b@github.com>

Derrick Stolee <derrickstolee@github.com> writes:

Show 9 quoted lines

> 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.

Show 5 quoted lines

>   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.

Show 5 quoted lines

>   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.