On 5/20/2025 9:30 AM, Junio C Hamano wrote: > Jacob Keller <jacob.e.keller@xxxxxxxxx> writes: > >> The git diff --no-index mode does not support pathspecs, and cannot >> limit the diff output in this way. Other diff programs such as GNU >> difftools have options for excluding paths based on a pattern match. > > True. > >> However, using git diff as a diff replacement has several advantages >> over many popular diff tools, including coloring moved lines, rename >> detections, and similar. > > I said this when we introduced "--no-index", but back when "git" was > still young, it would have helped a lot wider developer populations > if we didn't do "git diff --no-index" and instead donated these > features to "many popular diff tools". We however chose to be lazy > and selfish---those who want to use these features are better off > installing "git" even if they are not using it for version control, > only to use it as "better diff". > > People are still welcome to add "coloring moved lines, rename > detections and similar" to "many popular diff tools", but given that > "git" has become fairly popular and widely used, it may not matter > all that much any more that we were lazy and selfish ;-). > >> Teach git diff --no-index how to handle pathspecs to limit the >> comparisons. This will only be supported if both provided paths are >> directories. > > Good. If you are giving only two files, pathspec limiting would not > make much sense. If you are giving a file and a directory, lazily > give only F and D to compare F with D/F, that is essentially giving > only two files F and D/F, so pathspec limiting would not make much > sense, either. pathspec limited comparison would make sense only > when you are talking about two sets of files. > >> However, if we always passed DO_MATCH_LEADING_PATHSPEC, then we will >> incorrectly match in certain cases such as matching 'a/c' against >> ':(glob)**/d'. The match logic will see that a matches the leading part >> of the **/ and accept this even tho c doesn't match. The trick seems to >> be setting both DO_MATCH_LEADING_PATHSPEC and DO_MATCH_DIRECTORY when >> checking directories, but set neither of them when checking files. > > Sounds sensible. > >> Some other gotchas and open questions: >> >> 1) pathspecs must all come after the first two path arguments, you >> can't re-arrange them to come first. I'm treating them sort of like >> the treeish arguments to git diff-tree. > > Exactly. "git diff" proper is about comparing two sets of files, > either comparing two tree-ishes "git diff master next", comparing a > tree-ish and the index "git diff --cached HEAD", comparing a > tree-ish and the working tree files "git diff HEAD", or comparing > the index and the working tree files "git diff". It is a natural > extension that "git --no-index dirA dirB" compares contents of the > two directories. In all of these forms, it is common that the > comparison can be pathspec limited by giving pathspec elements as > the command line arguments at the end. > >> 2) The pathspecs are interpreted relative to the provided paths, and >> thus will always need to be specified as relative paths, and will be >> interpreted as relative to the root of the search for each path >> separately. > > Yes, that is not anything new or something we need to point out as > if it is any different from the "normal" pathspec. > >> 3) negative pathspecs have to be fully qualified from the root, i.e. >> ':(exclude)file' will only exclude 'a/file' and not 'a/b/file' >> unless you also use '(glob)' or similar. I think this matches the >> other pathspec support, but I an not 100% sure. > > I think that is correct "git ls-files :(exclude)po" does not exclude > git-gui/po, for example. > Sounds like you answered most of my open questions. I guess my remaining big question to you or others on the list is whether or not the overall algorithm makes sense, especially in regards to how we handle needing both DO_MATCH_DIRECTORY and DO_MATCH_LEADING_PATHSPEC, and in using two separate calls to parse_pathspec, one set for each path.
