Re: [PATCH v5 1/3] builtin/grep.c: add --sparse option

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 15, 2022, 02:57 UTC
Message-ID: <xmqqh719pcoo.fsf@gitster.g>
In-Reply-To: <CABPp-BF-z72=hY_Jf8h3g95s+wwZOsV_S=+dDNs_AVskQxoaTw@mail.gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 15 quoted lines

> ... I think all those other commands probably deserve a mode where they
> restrict output to the view associated with the user's cone.  I've
> brought that up before[1].  I was skeptical of making it the default,
> because it'd probably take a long time to implement it everywhere.
> Slowly changing defaults of all commands over many git releases seems
> like a poor strategy, but I'm afraid that's what it looks like we are
> doing here.
>
> I'm also worried that slowly changing the defaults without a
> high-level plan will lead to users struggling to figure out what
> flag(s) to pass.  Are we going to be stuck in a situation where users
> have to remember that for a dense search, they use one flag for `grep
> --cached`, a different one for  `grep [REVISION]`, no flag is needed
> for `diff [REVISION]`, but yet a different flag is needed for `git
> log`?

In short, the default should be "everywhere in tree, regardless of the current sparse-checkout settings", with commands opting into implementing "limit only to sparse-checkout settings" as an option, at least initially, with an eye to possibly flip the default later when all commands support that position but not before?

I think that is a reasonable position to take. I lean towards the default of limiting the operations to inside sparse cone(s) for all subcommands when all subcommands learn to be capable to do so, but I also agree that using that default for only for subcommands that have learned to do, which will happen over time, would be way too confusing for our users.

By the way, I briefly wondered if "limit to sparse-checkout setting" can be done by introducing a fake "attribute" and using the "attr" pathspec magic, but it may probably be a bad match, and separate option would be more appropriate.

Show 8 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.
>
> I still think the flag name of `--sparse` is totally backwards and
> highly confusing for the described behavior.

Yeah, regardless of which between "--sparse" and "--no-sparse" should be the default, I am in 100% agreement that "--sparse" meaning "affect things both inside and outside the sparse cones" is totally backwards.

How strongly ingrained is this UI mistake? I have a feeling that this may be something we still can undo and redo relatively easily, i.e. "--sparse" may be that "limit to sparse-checkout setting" option, not "--no-sparse".