Web lists-archives.com

Re: Deprecating git diff ..; dealing with other ranges




Hi,

On Mon, Mar 11, 2019 at 2:37 AM Denton Liu <liu.denton@xxxxxxxxx> wrote:
>
> Hello all,
>
> I was in the process of deprecating `git diff <commit>..<commit>` as
> discussed here[1]. However, I ran into a weird case that I'm not sure
> how to deal with.
>
> In t3430-rebase-merges.sh:382, we have the following test case which
> invokes git diff:
>
>         test_expect_success 'with --autosquash and --exec' '
>                 git checkout -b with-exec H &&
>                 echo Booh >B.t &&
>                 test_tick &&
>                 git commit --fixup B B.t &&
>                 write_script show.sh <<-\EOF &&
>                 subject="$(git show -s --format=%s HEAD)"
> =>              content="$(git diff HEAD^! | tail -n 1)"
>                 echo "$subject: $content"
>                 EOF
>                 test_tick &&
>                 git rebase -ir --autosquash --exec ./show.sh A >actual &&
>                 grep "B: +Booh" actual &&
>                 grep "E: +Booh" actual &&
>                 grep "G: +G" actual
>         '
>
> It gets caught in my attempt to only deprecate ..'s. Technically, it's
> undocumented behaviour and it only happens to work because git-diff
> accept ranges but it doesn't operate in an intuitive way.
>
> I was just wondering what we should do about this case? Should we
> deprecate all invocations of `git diff <range>` except for the special
> case of `git diff <commit>...<commit>`, or should we _only_ deprecate
> `git diff <commit>..<commit>` and allow all other forms of ranges, even
> though it was undocumented behaviour?

There's a few angles I can think of to view this from:

First, commit^! is somewhat of a degenerate "range"; it includes only
one commit and for non-merge commits, is equal to both
commit~1..commit and commit~1...commit.  The fact that those ranges
are equal means that "git diff commit~1..commit" and "git diff
commit~1...commit" will also give equal results, i.e. that this is not
a case where confusion between '..' and '...' will cause any problems
for the user.  (Admittedly, I'm ignoring usage of ^! with a merge
commit here; I've never seen anyone use it in such a case.)

Second, ^! is unlikely to cause confusion for users the way '..' vs
'...' will, because the syntax makes no sense with diff anyway.  It's
totally magical, and when I came across it being used with diff
instead of log, I had to test it out to determine what it did.  (I do
now find it handy and use it occasionally.)  It's fairly difficult to
explain to beginners -- I tried once and quickly gave up and used
longer but more straightforward alternatives.  So, IMO, this is only a
convenience syntax for experts, and a syntax that won't be confused
with other syntax out there, so there's no need to deprecate it.

Third, we have good reason to deprecate explicit usage of '..' with
git diff, but even with those reasons some folks probably aren't
convinced it's worth the effort.  I want to avoid expanding scope, for
fear of moving some people from the sidelines of "not worth the
effort" to "this is a bad idea".  So if there are other range syntax
cases used in the wild (maybe 'git diff merge_commit^@' ?!?), we
should just leave them be and allow them to continue working.


Hope that helps,
Elijah