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. 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 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 v1: * shortened first commit's message * renamed prepare function * removed A..B syntax from usage (and fixed typo) * added combined diff syntax to main documentation Note: I looked into adding special handling for rev^! syntax and it seems a bit messy. prepare_symdiff() could do this with its other analysis, and slide the decoded revisions around. Perhaps better, revision.c could insert the parent refs after the child, under control of a flag in the diff flags section of a rev_info. Chris Torek (3): t/t3430: avoid undefined git diff behavior git diff: improve A...B merge-base handling Documentation: tweak git diff help slightly Documentation/git-diff.txt | 21 ++++-- builtin/diff.c | 137 ++++++++++++++++++++++++++++++++----- t/t3430-rebase-merges.sh | 2 +- t/t4068-diff-symmetric.sh | 81 ++++++++++++++++++++++ 4 files changed, 220 insertions(+), 21 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-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-804/chris3torek/cleanup-diff-v2 Pull-Request: https://github.com/git/git/pull/804 Range-diff vs v1: 1: 414163bbc3c ! 1: 2ccaad645ff t/t3430: avoid undocumented git diff behavior @@ Metadata Author: Chris Torek <chris.torek@xxxxxxxxx> ## Commit message ## - t/t3430: avoid undocumented git diff behavior + t/t3430: avoid undefined git diff behavior - According to the documentation, "git diff" takes at most two commit-ish, - or an A..B style range, or an A...B style symmetric difference range. - The autosquash-and-exec test relied on "git diff HEAD^!", which works - fine for ordinary commits as the revision parse produces two commit-ish, - namely ^HEAD^ and HEAD. - - For merge commits, however, this test makes use of an undocumented - feature: the resulting revision parse has all the parents as UNINTERESTING - followed by the HEAD commit. This looks identical to a symmetric - diff parse, which lists the merge bases as UNINTERESTING, followed by - the A (UNINTERESTING) and B revs. So the diff winds up treating it - as one, using the first oid (i.e., HEAD^) and the last (i.e., HEAD). - The documentation, however, says nothing about this usage. - - Since diff actually just uses HEAD^ and HEAD, call for these directly - here. That makes it possible to improve the diff code's handling of - symmetric difference arguments. + The autosquash-and-exec test used "git diff HEAD^!" to mean + "git diff HEAD^ HEAD". Use these directly instead of relying + on the undefined but actual-current behavior of "HEAD^!". Signed-off-by: Chris Torek <chris.torek@xxxxxxxxx> 2: f7c8f094e02 ! 2: 100fa403477 git diff: improve A...B merge-base handling @@ builtin/diff.c: static int builtin_diff_files(struct rev_info *revs, int argc, c + * If the user provides a symmetric diff with no merge base, or + * more than one range, we do a usage-exit. + */ -+static void builtin_diff_symdiff(struct rev_info *rev, struct symdiff *sym) ++static void symdiff_prepare(struct rev_info *rev, struct symdiff *sym) +{ + int i, lcount = 0, rcount = 0, basecount = 0; + int lpos = -1, rpos = -1, basepos = -1; @@ builtin/diff.c: int cmd_diff(int argc, const char **argv, const char *prefix) } } -+ builtin_diff_symdiff(&rev, &sdiff); ++ symdiff_prepare(&rev, &sdiff); for (i = 0; i < rev.pending.nr; i++) { struct object_array_entry *entry = &rev.pending.objects[i]; struct object *obj = entry->item; 3: 9318365915c ! 3: b9b4c6f113d Documentation: tweak git diff help slightly @@ Metadata ## Commit message ## Documentation: tweak git diff help slightly - Update the manual page synopsis to include the two and three - dot notation. + Update the manual page synopsis to include the three-dot notation + and the combined-diff option Make "git diff -h" print the same usage summary as the manual - page synopsis. + page synopsis, minus the "A..B" form, which is now discouraged. + + Document the usage for producing combined commits. Signed-off-by: Chris Torek <chris.torek@xxxxxxxxx> ## Documentation/git-diff.txt ## @@ Documentation/git-diff.txt: SYNOPSIS + [verse] 'git diff' [<options>] [<commit>] [--] [<path>...] 'git diff' [<options>] --cached [<commit>] [--] [<path>...] - 'git diff' [<options>] <commit> <commit> [--] [<path>...] -+'git diff' [<options>] <commit>..<commit> [--] [<path>...] +-'git diff' [<options>] <commit> <commit> [--] [<path>...] ++'git diff' [<options>] <commit> [<commit>...] <commit> [--] [<path>...] +'git diff' [<options>] <commit>...<commit> [--] [<path>...] 'git diff' [<options>] <blob> <blob> 'git diff' [<options>] --no-index [--] <path> <path> + DESCRIPTION + ----------- + Show changes between the working tree and the index or a tree, changes +-between the index and a tree, changes between two trees, changes between +-two blob objects, or changes between two files on disk. ++between the index and a tree, changes between two trees, changes resulting ++from a merge, changes between two blob objects, or changes between two ++files on disk. + + 'git diff' [<options>] [--] [<path>...]:: + +@@ Documentation/git-diff.txt: two blob objects, or changes between two files on disk. + one side is omitted, it will have the same effect as + using HEAD instead. + ++'git diff' [<options>] <commit> [<commit>...] <commit> [--] [<path>...]:: ++ ++ This form is to view the results of a merge commit. The first ++ listed <commit> must be the merge itself; the remaining two or ++ more commits should be its parents. A convenient way to produce ++ the desired set of revisions is to use the {caret}@ suffix, i.e., ++ "git diff master master^@". This is equivalent to running "git ++ show --format=" on the merge commit, e.g., "git show --format= ++ master". ++ + 'git diff' [<options>] <commit>\...<commit> [--] [<path>...]:: + + This form is to view the changes on the branch containing +@@ Documentation/git-diff.txt: linkgit:git-difftool[1], + linkgit:git-log[1], + linkgit:gitdiffcore[7], + linkgit:git-format-patch[1], +-linkgit:git-apply[1] ++linkgit:git-apply[1], ++linkgit:git-show[1] + + GIT + --- ## builtin/diff.c ## @@ @@ builtin/diff.c -"git diff [<options>] [<commit> [<commit>]] [--] [<path>...]"; +"git diff [<options>] [<commit>] [--] [<path>...]\n" +" or: git diff [<options>] --cached [<commit>] [--] [<path>...]\n" -+" or: git diff [<options>] <commit> <commit>] [--] [<path>...]\n" -+" or: git diff [<options>] <commit>..<commit>] [--] [<path>...]\n" ++" or: git diff [<options>] <commit> [<commit>...] <commit> [--] [<path>...]\n" +" or: git diff [<options>] <commit>...<commit>] [--] [<path>...]\n" +" or: git diff [<options>] <blob> <blob>]\n" +" or: git diff [<options>] --no-index [--] <path> <path>]\n" -- gitgitgadget
