From: "Chris Torek via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Chris Torek <chris.torek@gmail.com>
Subject: [PATCH v2 0/3] improve git-diff documentation and A...B handling
Date: Tue, 09 Jun 2020 19:00:20 +0000 [thread overview]
Message-ID: <pull.804.v2.git.git.1591729224.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.804.git.git.1591661021.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
next prev parent reply other threads:[~2020-06-09 19:00 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-09 0:03 [PATCH 0/3] improve git-diff documentation and A...B handling Chris Torek via GitGitGadget
2020-06-09 0:03 ` [PATCH 1/3] t/t3430: avoid undocumented git diff behavior Chris Torek via GitGitGadget
2020-06-09 5:18 ` Junio C Hamano
2020-06-09 0:03 ` [PATCH 2/3] git diff: improve A...B merge-base handling Chris Torek via GitGitGadget
2020-06-09 5:40 ` Junio C Hamano
2020-06-12 13:38 ` Philip Oakley
2020-06-12 17:06 ` Junio C Hamano
2020-06-09 0:03 ` [PATCH 3/3] Documentation: tweak git diff help slightly Chris Torek via GitGitGadget
2020-06-09 5:45 ` Junio C Hamano
2020-06-09 19:00 ` Chris Torek via GitGitGadget [this message]
2020-06-09 19:00 ` [PATCH v2 1/3] t/t3430: avoid undefined git diff behavior Chris Torek via GitGitGadget
2020-06-09 19:00 ` [PATCH v2 2/3] git diff: improve A...B merge-base handling Chris Torek via GitGitGadget
2020-06-09 22:52 ` Junio C Hamano
2020-06-09 19:00 ` [PATCH v2 3/3] Documentation: tweak git diff help slightly Chris Torek via GitGitGadget
2020-06-09 23:01 ` Junio C Hamano
2020-06-11 15:15 ` [PATCH v3 0/3] improve git-diff documentation and A...B handling Chris Torek via GitGitGadget
2020-06-11 15:15 ` [PATCH v3 1/3] t/t3430: avoid undefined git diff behavior Chris Torek via GitGitGadget
2020-06-11 15:15 ` [PATCH v3 2/3] git diff: improve range handling Chris Torek via GitGitGadget
2020-06-11 15:51 ` Chris Torek
2020-06-11 15:15 ` [PATCH v3 3/3] Documentation: usage for diff combined commits Chris Torek via GitGitGadget
2020-06-12 16:19 ` [PATCH v4 0/3] improve git-diff documentation and A...B handling Chris Torek via GitGitGadget
2020-06-12 16:19 ` [PATCH v4 1/3] t/t3430: avoid undefined git diff behavior Chris Torek via GitGitGadget
2020-06-12 16:19 ` [PATCH v4 2/3] git diff: improve range handling Chris Torek via GitGitGadget
2020-06-12 18:31 ` Junio C Hamano
2020-06-12 19:25 ` Chris Torek
2020-06-12 16:20 ` [PATCH v4 3/3] Documentation: usage for diff combined commits Chris Torek via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.804.v2.git.git.1591729224.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=chris.torek@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).