[PATCH] warn use of "git diff A..B"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"git diff" (and "diff-tree") accepts a range notation "A..B" from the
command line to specify the two endpoints to be compared; the right way to
spell this would be "git diff A B".  This is merely a historical accident
that comes from the fact that "git log" family of commands and "git diff"
happens to share some code in their command line parsers.

It was fine for people who are used to seeing "git diff A..B" and silently
translating it to "git diff A B" in their head, but it made things hard
for new people.  It is easy to mistakenly think that "git diff A..B" has
some similarity with "git log A..B" (there isn't).

Indeed, "git log A..B" computes for the range "A..B" a set of commits that
are in B but not in A, and if one misunderstands "git diff" to somehow
magically work with ranges (it doesn't), it is more natural to expect that
"git diff A..B" might show a cumulative changes since B forked from A.

But that is not what the command gives (it gives the difference between
two endpoints and there is no history cosideration such as "B forked from
A").

New people can be trained not to say "git diff A..B" when they mean to
compare two endpoints with "git diff A B", and that would reduce the
confusion greatly.

Warn the use of "git diff A..B" syntax when "git diff A B" equally works
well, is shorter, and is much more clear what the command is comparing
(i.e. two endpoints).

The new code does not issue a warning against "git diff ..B" that is used
as a shorthand for "git diff HEAD B", and "git diff A.." that is used as a
shorthand for "git diff A HEAD", respectively.  These are shorter to type
and are often useful.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---

 * Cumulative effect can be had with "git diff $(git merge-base A B) B"
   and there is a short-hand for it "git diff A...B", but this change is
   about the two-dot notation, not three-dots.

 builtin/diff.c |   16 ++++++++++++++++
 revision.c     |    6 ++++--
 revision.h     |    1 +
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 86614d4..6d8028b 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -142,12 +142,28 @@ static int builtin_diff_index(struct rev_info *revs,
 	return run_diff_index(revs, cached);
 }
 
+static void check_useless_use_of_range(struct object_array_entry *ent)
+{
+	if (!(ent[0].item->flags & UNINTERESTING) ||
+	    ent[1].item->flags & UNINTERESTING)
+		return; /* not a range made by "A..B" notation */
+
+	if ((ent[0].name == dotdot_default_HEAD) ||
+	    (ent[1].name == dotdot_default_HEAD))
+		return;	/* "A.." or "..B" */
+
+	warning("Do not write 'git diff A..B' but write 'git diff A B'");
+	warning("diff is about two endpoints!");
+}
+
 static int builtin_diff_tree(struct rev_info *revs,
 			     int argc, const char **argv,
 			     struct object_array_entry *ent)
 {
 	if (argc > 1)
 		usage(builtin_diff_usage);
+
+	check_useless_use_of_range(ent);
 	diff_tree_sha1(ent[0].item->sha1, ent[1].item->sha1, "", &revs->diffopt);
 	log_tree_diff_flush(revs);
 	return 0;
diff --git a/revision.c b/revision.c
index e96c281..48a0a44 100644
--- a/revision.c
+++ b/revision.c
@@ -1008,6 +1008,8 @@ static void prepare_show_merge(struct rev_info *revs)
 	revs->limited = 1;
 }
 
+const char dotdot_default_HEAD[] = "HEAD";
+
 int handle_revision_arg(const char *arg, struct rev_info *revs,
 			int flags,
 			int cant_be_filename)
@@ -1030,9 +1032,9 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 		next += symmetric;
 
 		if (!*next)
-			next = "HEAD";
+			next = dotdot_default_HEAD;
 		if (dotdot == arg)
-			this = "HEAD";
+			this = dotdot_default_HEAD;
 		if (!get_sha1(this, from_sha1) &&
 		    !get_sha1(next, sha1)) {
 			struct commit *a, *b;
diff --git a/revision.h b/revision.h
index ae94860..27a233c 100644
--- a/revision.h
+++ b/revision.h
@@ -151,6 +151,7 @@ struct rev_info {
 /* revision.c */
 typedef void (*show_early_output_fn_t)(struct rev_info *, struct commit_list *);
 extern volatile show_early_output_fn_t show_early_output;
+extern const char dotdot_default_HEAD[];
 
 struct setup_revision_opt {
 	const char *def;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]