[PATCH v2 0/3] improve git-diff documentation and A...B handling

["Chris Torek via GitGitGadget" <gitgitgadget@gmail.com>]

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@gmail.com>
      
       ## 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@gmail.com>
      
 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@gmail.com>
      
       ## 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