Re: [PATCH v2 6/7] add: warn when pathspec only matches SKIP_WORKTREE entries

From: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Date: Mar 4, 2021, 21:26 UTC
Message-ID: <CAHd-oW4+4ccgLjoKqygSzqZXgk7Ljhe_ovSVSsJoJv=4kyv1eQ@mail.gmail.com>
In-Reply-To: <CABPp-BE9QA5RAwdebmAyK3b3wh1mGE+NF7sPS5RCf4FogYBV7A@mail.gmail.com>

On Thu, Mar 4, 2021 at 2:22 PM Elijah Newren <newren@gmail.com> wrote:

Show 41 quoted lines

>
> On Thu, Mar 4, 2021 at 7:23 AM Matheus Tavares
> <matheus.bernardino@usp.br> wrote:
> >
> > On Wed, Feb 24, 2021 at 3:50 AM Elijah Newren <newren@gmail.com> wrote:
> > >
> > > Hmm...here's an interesting command sequence:
> > >
> > > git init lame
> > > cd lame
> > > mkdir baz
> > > touch baz/tracked
> > > git add baz/tracked
> > > git update-index --skip-worktree baz/tracked
> > > rm baz/tracked.  # But leave the empty directory!
> > > echo baz >.gitignore
> > > git add --ignore-missing --dry-run baz
> > >
> > >
> > > Reports the following:
> > > """
> > > The following pathspecs only matched index entries outside the current
> > > sparse checkout:
> > > baz
> > > hint: Disable or modify the sparsity rules if you intend to update such entries.
> > > hint: Disable this message with "git config advice.updateSparsePath false"
> > > The following paths are ignored by one of your .gitignore files:
> > > baz
> > > hint: Use -f if you really want to add them.
> > > hint: Turn this message off by running
> > > hint: "git config advice.addIgnoredFile false"
> > > """
> >
> >[...] Perhaps should we just rephrase the sparse warning
> > to remove the "only" part (and don't try to avoid the double warnings)?
> > I'm open to any suggestions on alternative wordings or ideas :)
>
> The only thing that I think was problematic about the double warning
> was the contradiction between them due to the use of "only" in the
> first; if we remove that, I think printing two warnings is perfectly
> fine.  So, I'm in favor of just rephrasing as you suggest.

Ok, let's do that! I may be overthinking this, but would it be enough to say "The following pathspecs match index entries outside the current sparse checkout"?

This is not wrong per-se, but I think it doesn't sound as informative. I mean, another add/rm execution could also have pathspecs that match sparse entries, and yet no warning is displayed because they also match non-sparse entries.

I haven't thought of a better alternative yet, but I was considering something along "The following pathspecs won't update any paths, but they match index entries outside the current sparse checkout". (The idea is that "won't update any paths" include both pathspecs that only match sparse paths and the ones that match both sparse and ignored paths, as neither will be updated.)