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. > -`git diff [<options>] --no-index [--] <path> <path>`:: > +`git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]`:: This is a bit unfortunate. The disambiguating "--" should ideally be between the "things to be compared" and the pathspec, as the former corresponds to <rev> in the normal "git diff" invocation. > + ... If both > + paths point to directories, additional pathspecs may be > + provided. These will limit the files included in the > + difference. All such pathspecs must be relative as they > + apply to both sides of the diff. "as they" -> "and they"? > +test_expect_success 'diff --no-index with pathspec' ' > + test_expect_code 1 git diff --no-index a b 1 >actual && > + cat >expect <<-EOF && > + diff --git a/a/1 b/a/1 > + deleted file mode 100644 > + index d00491f..0000000 > + --- a/a/1 > + +++ /dev/null > + @@ -1 +0,0 @@ > + -1 > + EOF > + test_cmp expect actual > +' If you use --name-only or --name-status would the test become simpler? > + > +test_expect_success 'diff --no-index with pathspec no matches' ' > + test_expect_code 0 git diff --no-index a b missing > +' OK. > +test_expect_success 'diff --no-index with negative pathspec' ' > + test_expect_code 1 git diff --no-index a b ":!2" >actual && > + cat >expect <<-EOF && > + diff --git a/a/1 b/a/1 > + deleted file mode 100644 > + index d00491f..0000000 > + --- a/a/1 > + +++ /dev/null > + @@ -1 +0,0 @@ > + -1 > + EOF > + test_cmp expect actual > +' OK. All other tests also look sensible. Thanks.
