git diff -h help is succinct, but perhaps too much so. The symmetric-diff syntax, git diff A...B, is defined by the documentation to compare the merge base of A and B to commit B. It does so just fine when there is a merge base. It compares A and B directly if there is no merge base, and it is overly forgiving of bad arguments after which it can produce nonsensical diffs. It also behaves badly with other odd/incorrect usages, such as git diff A...B C..D. The first patch simply adjusts a test that will fail if the second patch is accepted. The second patch adds special handling for the symmetric and range diff syntax so that the option parsing works, plus a small test suite. The third patch updates the documentation, including adding a section for combined commits, and makes the help output more verbose (to match the SYNOPSIS and provide common diff options like git-diff-files, for instance). Changes since v3: * correct > / >= goof * fix test nit per Philip Oakley Chris Torek (3): t/t3430: avoid undefined git diff behavior git diff: improve range handling Documentation: usage for diff combined commits Documentation/git-diff.txt | 20 ++++-- builtin/diff.c | 132 +++++++++++++++++++++++++++++++++---- t/t3430-rebase-merges.sh | 2 +- t/t4068-diff-symmetric.sh | 91 +++++++++++++++++++++++++ 4 files changed, 226 insertions(+), 19 deletions(-) create mode 100755 t/t4068-diff-symmetric.sh base-commit: 20514004ddf1a3528de8933bc32f284e175e1012 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-804%2Fchris3torek%2Fcleanup-diff-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-804/chris3torek/cleanup-diff-v4 Pull-Request: https://github.com/git/git/pull/804 Range-diff vs v3: 1: 2ccaad645ff = 1: 2ccaad645ff t/t3430: avoid undefined git diff behavior 2: 60aed3f9d65 ! 2: 4fa6fba33b3 git diff: improve range handling @@ builtin/diff.c: static int builtin_diff_files(struct rev_info *revs, int argc, c + basecount++; + break; /* do mark all bases */ + case REV_CMD_LEFT: -+ if (lpos > 0) ++ if (lpos >= 0) + usage(builtin_diff_usage); + lpos = i; + if (obj->flags & SYMMETRIC_LEFT) { @@ builtin/diff.c: static int builtin_diff_files(struct rev_info *revs, int argc, c + } + continue; + case REV_CMD_RIGHT: -+ if (rpos > 0) ++ if (rpos >= 0) + usage(builtin_diff_usage); + rpos = i; + continue; /* don't mark B */ @@ t/t4068-diff-symmetric.sh (new) +. ./test-lib.sh + +# build these situations: -+# - normal merge with one merge base (b1...b2); -+# - criss-cross merge ie 2 merge bases (b1...master); -+# - disjoint subgraph (orphan branch, b3...master). ++# - normal merge with one merge base (br1...b2r); ++# - criss-cross merge ie 2 merge bases (br1...master); ++# - disjoint subgraph (orphan branch, br3...master). +# +# B---E <-- master +# / \ / 3: a7da92cd635 = 3: d7bc9aca44b Documentation: usage for diff combined commits -- gitgitgadget
