threads / discuss / 50710

Deprecating git diff ..; dealing with other ranges

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

## tl;dr

14 messages between Mar 11, 2019 and Mar 18, 2019.

replies: 13people: 9as markdown or json

Denton Liu· Mar 11, 2019, 09:37 UTC · lore

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?

Thanks,

Denton

[1]: https://public-inbox.org/git/xmqqmumy6mxe.fsf@gitster-ct.c.googlers.com/

Johannes Schindelin· Mar 11, 2019, 13:19 UTC · re: Denton Liu · lore

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

Hi Denton,

On Mon, 11 Mar 2019, Denton Liu wrote:

Show 27 quoted lines

> 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 beg to differ. `git diff <commit>^!` does exactly what I want: it shows the diff between the commit's first parent and the commit.

And I would not necessarily call this a "range". It is a short-hand. Can't you allow short-hands in `git diff`, and disallow only ranges that have an explicit `..` in them? You might need to record that somewhere, but I think that should be easy enough.

Ciao, Johannes

Show 13 quoted lines

> 
> 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?
> 
> Thanks,
> 
> Denton
> 
> [1]: https://public-inbox.org/git/xmqqmumy6mxe.fsf@gitster-ct.c.googlers.com/
>

Elijah Newren· Mar 11, 2019, 15:34 UTC · re: Denton Liu · lore

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

Hi,

On Mon, Mar 11, 2019 at 2:37 AM Denton Liu <liu.denton@gmail.com> wrote:

Show 36 quoted lines

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

Junio C Hamano· Mar 12, 2019, 07:17 UTC · re: Denton Liu · lore

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

Denton Liu <liu.denton@gmail.com> writes:

> I was in the process of deprecating `git diff <commit>..<commit>` as
> discussed here[1].
>
> [1]: https://public-inbox.org/git/xmqqmumy6mxe.fsf@gitster-ct.c.googlers.com/

I didn't (and don't) advocate such a deprecation, FWIW, in that message. I simply do not think it is worth the cost.

I however think it may be worth making sure that our docs do not encourage "diff A..B" and teach "diff A B" when comparing two endpoints. That can be done without changing anything in the code.

Andreas Schwab· Mar 12, 2019, 17:24 UTC · re: Junio C Hamano · lore

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

On Mär 12 2019, Junio C Hamano <gitster@pobox.com> wrote:

> I however think it may be worth making sure that our docs do not
> encourage "diff A..B" and teach "diff A B" when comparing two
> endpoints.  That can be done without changing anything in the code.

The nice thing about "diff A..B" is that you can c&p the output from the fetch run without the need to edit it.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Ævar Arnfjörð Bjarmason· Mar 12, 2019, 21:01 UTC · re: Andreas Schwab · lore

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

On Tue, Mar 12 2019, Andreas Schwab wrote:

Show 8 quoted lines

> On Mär 12 2019, Junio C Hamano <gitster@pobox.com> wrote:
>
>> I however think it may be worth making sure that our docs do not
>> encourage "diff A..B" and teach "diff A B" when comparing two
>> endpoints.  That can be done without changing anything in the code.
>
> The nice thing about "diff A..B" is that you can c&p the output from the
> fetch run without the need to edit it.

Not to shoot down this effort, just to add another similar thing I do regularly for ff-branches:

 1. Copy/paste A..B fetch output
 2. git log A..B
 3. ^log^diff

I.e. I just need to tell my terminal to re-run the same "log" command with "diff" instead of "log".

Of course as covered in the linked thread it doesn't work for some (non-ff) cases, and I'll sometimes end up cursing it and swapping around ".." for "..." with log/diff.

Johannes Sixt· Mar 13, 2019, 07:01 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Am 12.03.19 um 22:01 schrieb Ævar Arnfjörð Bjarmason:

Show 25 quoted lines

> 
> On Tue, Mar 12 2019, Andreas Schwab wrote:
> 
>> On Mär 12 2019, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>> I however think it may be worth making sure that our docs do not
>>> encourage "diff A..B" and teach "diff A B" when comparing two
>>> endpoints.  That can be done without changing anything in the code.
>>
>> The nice thing about "diff A..B" is that you can c&p the output from the
>> fetch run without the need to edit it.
> 
> Not to shoot down this effort, just to add another similar thing I do
> regularly for ff-branches:
> 
>  1. Copy/paste A..B fetch output
>  2. git log A..B
>  3. ^log^diff
> 
> I.e. I just need to tell my terminal to re-run the same "log" command
> with "diff" instead of "log".
> 
> Of course as covered in the linked thread it doesn't work for some
> (non-ff) cases, and I'll sometimes end up cursing it and swapping around
> ".." for "..." with log/diff.

Still, your and Andreas's use-case is reason enough *not* to deprecate diff A..B, IMO.

We wouldn't be able to remove .. wholesale anyway because diff A.. and diff ..B, where the missing endpoints are filled in with HEAD would have to live on.

-- Hannes

Elijah Newren· Mar 18, 2019, 17:07 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

On Tue, Mar 12, 2019 at 2:01 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 23 quoted lines

> On Tue, Mar 12 2019, Andreas Schwab wrote:
> > On Mär 12 2019, Junio C Hamano <gitster@pobox.com> wrote:
> >
> >> I however think it may be worth making sure that our docs do not
> >> encourage "diff A..B" and teach "diff A B" when comparing two
> >> endpoints.  That can be done without changing anything in the code.
> >
> > The nice thing about "diff A..B" is that you can c&p the output from the
> > fetch run without the need to edit it.
>
> Not to shoot down this effort, just to add another similar thing I do
> regularly for ff-branches:
>
>  1. Copy/paste A..B fetch output
>  2. git log A..B
>  3. ^log^diff
>
> I.e. I just need to tell my terminal to re-run the same "log" command
> with "diff" instead of "log".
>
> Of course as covered in the linked thread it doesn't work for some
> (non-ff) cases, and I'll sometimes end up cursing it and swapping around
> ".." for "..." with log/diff.

Doesn't this somewhat imply that although you use diff A..B here for convenience, that it's actually wrong since what you really want is A...B? Or said another way, the end goal of deprecating "diff "A..B" then later reinstating "diff A..B" to mean the same thing as "diff A...B" would actually be better even for your usecase?

Of course, switching to the removal period may just be too painful for too many folks since there are obviously people that use it, but I just want to see if I'm understanding correctly here.

Michal Suchánek· Mar 18, 2019, 17:11 UTC · re: Elijah Newren · lore

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

On Mon, 18 Mar 2019 10:07:08 -0700 Elijah Newren <newren@gmail.com> wrote:

Show 31 quoted lines

> On Tue, Mar 12, 2019 at 2:01 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> > On Tue, Mar 12 2019, Andreas Schwab wrote:  
> > > On Mär 12 2019, Junio C Hamano <gitster@pobox.com> wrote:
> > >  
> > >> I however think it may be worth making sure that our docs do not
> > >> encourage "diff A..B" and teach "diff A B" when comparing two
> > >> endpoints.  That can be done without changing anything in the code.  
> > >
> > > The nice thing about "diff A..B" is that you can c&p the output from the
> > > fetch run without the need to edit it.  
> >
> > Not to shoot down this effort, just to add another similar thing I do
> > regularly for ff-branches:
> >
> >  1. Copy/paste A..B fetch output
> >  2. git log A..B
> >  3. ^log^diff
> >
> > I.e. I just need to tell my terminal to re-run the same "log" command
> > with "diff" instead of "log".
> >
> > Of course as covered in the linked thread it doesn't work for some
> > (non-ff) cases, and I'll sometimes end up cursing it and swapping around
> > ".." for "..." with log/diff.  
> 
> Doesn't this somewhat imply that although you use diff A..B here for
> convenience, that it's actually wrong since what you really want is
> A...B?  Or said another way, the end goal of deprecating "diff "A..B"
> then later reinstating "diff A..B" to mean the same thing as "diff
> A...B" would actually be better even for your usecase?

I usually mean diff A..B in this case.

Thanks

Michal

Elijah Newren· Mar 18, 2019, 18:51 UTC · re: Michal Suchánek · lore

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

On Mon, Mar 18, 2019 at 10:11 AM Michal Suchánek <msuchanek@suse.de> wrote:

Show 37 quoted lines

>
> On Mon, 18 Mar 2019 10:07:08 -0700
> Elijah Newren <newren@gmail.com> wrote:
>
> > On Tue, Mar 12, 2019 at 2:01 PM Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> > > On Tue, Mar 12 2019, Andreas Schwab wrote:
> > > > On Mär 12 2019, Junio C Hamano <gitster@pobox.com> wrote:
> > > >
> > > >> I however think it may be worth making sure that our docs do not
> > > >> encourage "diff A..B" and teach "diff A B" when comparing two
> > > >> endpoints.  That can be done without changing anything in the code.
> > > >
> > > > The nice thing about "diff A..B" is that you can c&p the output from the
> > > > fetch run without the need to edit it.
> > >
> > > Not to shoot down this effort, just to add another similar thing I do
> > > regularly for ff-branches:
> > >
> > >  1. Copy/paste A..B fetch output
> > >  2. git log A..B
> > >  3. ^log^diff
> > >
> > > I.e. I just need to tell my terminal to re-run the same "log" command
> > > with "diff" instead of "log".
> > >
> > > Of course as covered in the linked thread it doesn't work for some
> > > (non-ff) cases, and I'll sometimes end up cursing it and swapping around
> > > ".." for "..." with log/diff.
> >
> > Doesn't this somewhat imply that although you use diff A..B here for
> > convenience, that it's actually wrong since what you really want is
> > A...B?  Or said another way, the end goal of deprecating "diff "A..B"
> > then later reinstating "diff A..B" to mean the same thing as "diff
> > A...B" would actually be better even for your usecase?
>
> I usually mean diff A..B in this case.

Thanks for the feedback; somewhat odd that you and Ævar have the same usecase but want the opposite meaning, but good to know nonetheless.

Ævar Arnfjörð Bjarmason· Mar 18, 2019, 17:59 UTC · re: Elijah Newren · lore

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

On Mon, Mar 18 2019, Elijah Newren wrote:

Show 29 quoted lines

> On Tue, Mar 12, 2019 at 2:01 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> On Tue, Mar 12 2019, Andreas Schwab wrote:
>> > On Mär 12 2019, Junio C Hamano <gitster@pobox.com> wrote:
>> >
>> >> I however think it may be worth making sure that our docs do not
>> >> encourage "diff A..B" and teach "diff A B" when comparing two
>> >> endpoints.  That can be done without changing anything in the code.
>> >
>> > The nice thing about "diff A..B" is that you can c&p the output from the
>> > fetch run without the need to edit it.
>>
>> Not to shoot down this effort, just to add another similar thing I do
>> regularly for ff-branches:
>>
>>  1. Copy/paste A..B fetch output
>>  2. git log A..B
>>  3. ^log^diff
>>
>> I.e. I just need to tell my terminal to re-run the same "log" command
>> with "diff" instead of "log".
>>
>> Of course as covered in the linked thread it doesn't work for some
>> (non-ff) cases, and I'll sometimes end up cursing it and swapping around
>> ".." for "..." with log/diff.
>
> Doesn't this somewhat imply that although you use diff A..B here for
> convenience, that it's actually wrong since what you really want is
> A...B?

Yeah "..." would be more correct, but usually when I do this it's for things where the LHS hasn't diverged.

> Or said another way, the end goal of deprecating "diff "A..B"
> then later reinstating "diff A..B" to mean the same thing as "diff
> A...B" would actually be better even for your usecase?

Yeah, it would be. Sometimes I need to go back and tweak it to be "..." instead of ".." now. When the two have different behavior I usually want the former.

> Of course, switching to the removal period may just be too painful for
> too many folks since there are obviously people that use it, but I
> just want to see if I'm understanding correctly here.

I think we'd ideally migrate, but the conversion might be too painful. Junio doesn't seem excited in <xmqqmum0h88n.fsf@gitster-ct.c.googlers.com>

I'm not claiming that this ^log^diff use-case is common or anything, just noting a use-case I do use occasionally and mostly works now.

Duy Nguyen· Mar 13, 2019, 01:20 UTC · re: Andreas Schwab · lore

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

On Wed, Mar 13, 2019 at 12:26 AM Andreas Schwab <schwab@linux-m68k.org> wrote:

Show 9 quoted lines

>
> On Mär 12 2019, Junio C Hamano <gitster@pobox.com> wrote:
>
> > I however think it may be worth making sure that our docs do not
> > encourage "diff A..B" and teach "diff A B" when comparing two
> > endpoints.  That can be done without changing anything in the code.
>
> The nice thing about "diff A..B" is that you can c&p the output from the
> fetch run without the need to edit it.

I do this copy&paste too though I think in this case we could improve something to not copy&paste in the first place.

A..B from fetch is the same as branch@{1}..branch. If we have some shortcut similar to ^! but for reflog, that would be perfect (the branch part does not require much typing with tab completion).

-- 
Duy

Andreas Schwab· Mar 13, 2019, 18:12 UTC · re: Duy Nguyen · lore

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

On Mär 13 2019, Duy Nguyen <pclouds@gmail.com> wrote:

> A..B from fetch is the same as branch@{1}..branch. If we have some
> shortcut similar to ^! but for reflog, that would be perfect (the
> branch part does not require much typing with tab completion).

Tab completion is much slower than c&p, especially if the completion is ambigous.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Junio C Hamano· Mar 12, 2019, 07:22 UTC · re: Denton Liu · lore

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

Denton Liu <liu.denton@gmail.com> writes:

Show 5 quoted lines

> =>		content="$(git diff HEAD^! | tail -n 1)"
> ...
> 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.

It reuses the same notation as that is used for ranges, but makes the notation mean something entirely different from ranges, because "diff" is about two endpoints.

And "git diff ^A B" (or "git diff B ^A") works like "git diff A B". So does "git diff A^!", for a single-parent commit A, work like "git diff A^ A".