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

From: Jay Soffian <jaysoffian@gmail.com>
Date: Mar 24, 2011, 18:11 UTC
Message-ID: <AANLkTikjN_90UApfD2rRksDk8qXyE6pJHhd00Aw-Qe7Y@mail.gmail.com>
In-Reply-To: <7vei5xcxzm.fsf@alter.siamese.dyndns.org>

On Wed, Mar 23, 2011 at 5:45 PM, Junio C Hamano <gitster@pobox.com> wrote:

> 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.

I think this is ill-advised because...

> 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).

... this is not consistent ...

> 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.

...with this.

I don't think telling newbies "Use diff A B instead of diff A..B, but A.. and ..B are okay" reduces confusion any. i.e., this makes more sense to me:

(1) One sided '..' implies HEAD on the other side with both git log and git diff (2) "diff A..B" is the same as "diff A B"

Therefor:

- "diff A.." is the same as "diff A..HEAD" (1) is the same as "diff A HEAD" (2).
- "diff ..B" is the same as "diff HEAD..B" (1) is the same as "diff HEAD B" (2).

Also, the fact that a one-sided '..' implies HEAD on the other side

Show 14 quoted lines

> +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!");
> +}
> +

I use diff A..B all the time, because I've often just used log A..B, or I'm about to, and it's one less part of the command line to change. Please make this warning squelch-able at least.

j.