From: Jacob Keller <jacob.keller@gmail.com>
Date: Thu, 15 May 2025 16:27:45 GMT
Subject: Re: [PATCH RFC] diff --no-index: teach option to exclude files by pattern
Message-ID: <CA+P7+xqg3S0q=n3nrTUJJuYicooDm83Q32AkpzRt1u7rH3n3Pw@mail.gmail.com>
In-Reply-To: <xmqqzffe7vbh.fsf@gitster.g>

On Wed, May 14, 2025 at 2:10 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
> > From: Jacob Keller <jacob.keller@gmail.com>
> >
> > Teach git diff --no-index the ability to exclude files by wildmatch
> > pattern when recursing through directories. The '--exclude' option
> > builds up a string list containing the patterns. These are checked with
> > wildmatch() in the read_directory_contents function. If any pattern
> > matches, then the file is not included in the directory contents.
>
> A quite natural question that comes to mind is:
>
>     How would we do this for the normal "git diff" that is not the
>     bolted on '--no-index' mode?
>
> but ...
>
> > The --exclude option is only supported by the --no-index mode. Standard
> > diff modes support negative pathspecs which is more powerful. I tried to
> > see if there was a way to add support for negative pathspecs themselves,
> > but haven't yet figured out if this is possible.
>
> ... of course you have thought about it already.  I do agree with
> you that we should figure out how and teach this mode to also use
> pathspec, not necessarily only the negative ones but positive ones.
>

Sure, though I think we might need either an option or some other way
to distinguish pathspec vs the existing non-pathspec mode.

> After all,
>
>     $ git diff --no-index [<option>...] dirA dirB
>
> is like running
>
>     $ diff -r [<option>...] dirA dirB
>
> after preparing these two directories like so:
>
>     $ git archive revA | ( mkdir dirA && tar Cxf dirA - )
>     $ git archive revB | ( mkdir dirB && tar Cxf dirB - )
>
> Hence it is natural for users to expect that anything you can do
> with
>
>     $ git diff revA revB
>
> should be doable, in
>
>     $ git diff --no-index dirA dirB
>
> and vice versa.  And as you said, when comparing two revisions,
> you'd use pathspec for this kind of thing.
>
>     $ git diff revA revB -- Documentation/ t/ ':!po/'
>
> So, I pretty much agree with the need to be able to exclude some
> parts of the tree(s) from comparison in "diff --no-index" mode, but
> I doubt it is a good idea to tell what to exclude the "--no-index"
> mode in a completely different way.
>

Right. My main issue was that pathspec seemed to have a bunch of stuff
baked into assuming it has a repository.

> The last time I looked at it, I got an impression that the command
> line argument parsing of "git diff --no-index" was messy (which is
> sort of inevitable, since unlike the normal "git diff", it can
> compare more than just two "collections"---it can take two paths to
> regular files, for example, and in such a case pathspec arguments
> can play no role), so teaching it pathspec parsing might be a bit of
> work, though.
>

Right. It currently requires finding two paths to compare, and some
DWIM logic to make directory and file comparisons work.

pathspec capability is about specifying which things to include or
exclude from a given search. Hmm..

Actually, I think I have a path forward:

we teach git diff --no-index to treat the first 2 arguments as they
are now: pointers to the things to compare.

We check if either or both of those are directories. If they aren't,
then additional arguments won't be accepted.

If we have at least one directory, then instead of rejecting commands
with >2 arguments, we interpret any remaining arguments as pathspecs,
which apply to any directory path provided. These can limit the search
when scanning through a directory, so both positive and negative ones
would apply in the same say.

I guess the one weirdness is that pathspecs must come after the first
2 arguments, since we need to find 2 paths first. But this matches the
way that treeish must come first in git diff-tree -r takes treeish and
then pathspecs, and you can't re-order them arbitrarily either.

Does this sound like a reasonable extension to the existing 2 argument
form of git diff --no-index?

> Thanks for starting an interesting topic.