Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH] Make 'diff C^!' show the same diff as 'show C'

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:17

Thomas Rast [off-list ref] writes:
* If C has no parents, setup_revision() turns C^! into simply C.  This
  meant that 'git diff C^!' compared the current worktree to C, which
  is certainly not what the user asked for.

* Otherwise setup_revision puts C itself last, i.e., the rev.pending
  are ^C^1 ... ^C^N C.  So the first revision is uninteresting and in
  the case of exactly two parents, the symmetric difference revspec
  (diff A...B) case fired, and compared C only to C^1 (instead of
  showing a combined diff).
I actually have a vague recollection that this ugly syntax C^! (and I am
entitled to call it ugly as it was my invention) was advertised as a way
to get "diff C^..C", not the combined diff, iow, this could be deliberate
and people may depend on it.

In that light, I would say the first one (showing the root commit as root)
may be a good change, but the latter one is moderately iffy.
quoted hunk
@@ -292,6 +310,17 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	/* Otherwise, we are doing the usual "git" diff */
 	rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
 
+	for (i = 1; i < argc; i++) {
+		if (prefixcmp(argv[i], "--")) {
+			if (strstr(argv[i], "..."))
+				mode = DIFF_MODE_SYMMETRIC;
+			else if (strstr(argv[i], "^!"))
+				mode = DIFF_MODE_SHOW;
+		} else if (!strcmp(argv[i], "--")) {
+			break;
+		}
+	}
+
This is too ugly beyond words.  

We already mark the left side commit "..." with SYMMETRIC_LEFT bit, so you
should be able to detect it from the setup_revisions() result.  If we were
to formerly add some special meaning (other than being a short-hand of
^C^n C) to the ugly C^! syntax, I would suggest marking the result of in a
similar way to allow you to detect it from the result.

But I do mean moderately strong negativeness when I say "if we _were_"
above.

As far as I recall, there were only two reasons C^! was invented for
(actually, one that was invented for, and another that was found to be
useful).

One was so that you can say "The traversal should stop around this commit,
but I want the commit itself to be included in the result".

	$ git log v1.6.3^! v1.6.3.1

This is much less useful than it used to be back when C^! was invented, as
we can ask for --boundary these days.

The other utility later found was "to view diff with its first parent".  I
think it is a useful mode of operation, and we should add a simpler way to
ask for it to "git show", as people often ask to view a merge not in the
combined way, but a simple diff relative to the merged-to commit, to get
an overview of the work done by the side branch.

So if anything, I'm actually for deprecating C^! syntax and removing it
before we hit 1.7.0.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help