BreakingChanges: say that `git diff X..Y` syntax will be removed in 3.0
PATCH 1/1 BreakingChanges: say that `git diff X..Y` syntax will be removed in 3.0
From: Martin von Zweigbergk <[email protected]>
The git diff X..Y syntax is quite misleading because it looks like it shows the diff of the commits in the X..Y range but it actually shows the diff from X to Y. IMO, if that syntax is supported, it should show a diff from the merge base of X and Y to Y. I hope Git 3.0 is a good time to remove support for the current syntax and semantics. Then we can perhaps add the syntax back later with less surprising semantics.
---
BreakingChanges: say that git diff X..Y syntax will be removed in 3.0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1989%2Fmartinvonz%2Fmz%2Fwtmnpolouvvz-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1989/martinvonz/mz/wtmnpolouvvz-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1989
Documentation/BreakingChanges.adoc | 4 ++++ 1 file changed, 4 insertions(+)
Documentation/BreakingChanges.adoc +4 -0
@@ -114,6 +114,10 @@ applications and forges. + There is no plan to deprecate the "sha1" object format at this point in time. + +Support for "git diff X..Y" syntax will be removed. Use "git diff X Y" instead. +This will open up the syntax for a more consistent interpretation of +"git diff $(git merge-base X Y) Y". ++ Cf. <[email protected]>, <[email protected]>, <CA+EOSBncr=4a4d8n9xS4FNehyebpmX8JiUwCsXD47EQDE+DiUQ@mail.gmail.com>.
base-commit: 143f58ef7535f8f8a80d810768a18bdf3807de26
Signature · 1 line
gitgitgadget
"Martin von Zweigbergk via GitGitGadget" <[email protected]>
Quoted 10 lines · writes:
From: Martin von Zweigbergk <[email protected]>
The
git diff X..Ysyntax is quite misleading because it looks like it shows the diff of the commits in the X..Y range but it actually shows the diff from X to Y. IMO, if that syntax is supported, it should show a diff from the merge base of X and Y to Y. I hope Git 3.0 is a good time to remove support for the current syntax and semantics. Then we can perhaps add the syntax back later with less surprising semantics.
Quoted 3 lines
Signed-off-by Martin von Zweigbergk <[email protected]>---
BreakingChanges: say that git diff X..Y syntax will be removed in 3.0
I like it in prinicple and I do wish that we didn't do the lazy thing when we did the command line parser for "git diff" (we had revision range parser, so we just reused it instead of doing our own for "git diff"). But real life may bite us back.
In any case, a declaration that does not come with code changes that are protected by WITH_BREAKING_CHANGES CPP macro is a patch that is not quite ready to be applied.
Quoted 24 lines
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1989%2Fmartinvonz%2Fmz%2Fwtmnpolouvvz-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1989/martinvonz/mz/wtmnpolouvvz-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1989Documentation/BreakingChanges.adoc | 4 ++++ 1 file changed, 4 insertions(+)Documentation/BreakingChanges.adoc +4
-0@@ -114,6 +114,10 @@ applications and forges. + There is no plan to deprecate the "sha1" object format at this point in time. + +Support for "git diff X..Y" syntax will be removed. Use "git diff X Y" instead. +This will open up the syntax for a more consistent interpretation of +"git diff $(git merge-base X Y) Y". ++ Cf. <[email protected]>, <[email protected]>, <CA+EOSBncr=4a4d8n9xS4FNehyebpmX8JiUwCsXD47EQDE+DiUQ@mail.gmail.com>.base-commit:
143f58ef7535f8f8a80d810768a18bdf3807de26
Quoted 26 lines · Martin von Zweigbergk via GitGitGadget
From: Martin von Zweigbergk <[email protected]>
The
git diff X..Ysyntax is quite misleading because it looks like it shows the diff of the commits in the X..Y range but it actually shows the diff from X to Y. IMO, if that syntax is supported, it should show a diff from the merge base of X and Y to Y. I hope Git 3.0 is a good time to remove support for the current syntax and semantics. Then we can perhaps add the syntax back later with less surprising semantics.Signed-off-by Martin von Zweigbergk <[email protected]>---
BreakingChanges: say that git diff X..Y syntax will be removed in 3.0Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1989%2Fmartinvonz%2Fmz%2Fwtmnpolouvvz-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1989/martinvonz/mz/wtmnpolouvvz-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1989Documentation/BreakingChanges.adoc | 4 ++++ 1 file changed, 4 insertions(+)Documentation/BreakingChanges.adoc +0
-0
Documentation/BreakingChanges.adoc applications and forges. · line 119
Quoted 6 lines
Documentation/BreakingChanges.adoc +3
-0@@ -114,6 +114,10 @@ applications and forges. + There is no plan to deprecate the "sha1" object format at this point in time. + +Support for "git diff X..Y" syntax will be removed. Use "git diff X Y" instead. +This will open up the syntax for a more consistent interpretation of +"git diff $(git merge-base X Y) Y".
I feel like this is going to break a whole lot of existing scripts and probably more than a few forges as well. It seems especially bad that we would add it back in the future with a completely different meaning, since we'll have some people that use 10-year LTS distros that go from, say, Git 2.51 to Git 3.xx, where the latter reintroduces the syntax with different semantics.
We've never really changed the meaning of things like revisions or revision-adjacent code in the past and I think those kinds of things we're pretty much stuck with forever. With that in mind, I don't think this is a good idea.
Signature · 2 lines
brian m. carlson (they/them) Toronto, Ontario, CA
Quoted 14 lines · Junio C Hamano
"Martin von Zweigbergk via GitGitGadget" <[email protected]>
Quoted 10 lines · writes:
From: Martin von Zweigbergk <[email protected]>
The
git diff X..Ysyntax is quite misleading because it looks like it shows the diff of the commits in the X..Y range but it actually shows the diff from X to Y. IMO, if that syntax is supported, it should show a diff from the merge base of X and Y to Y. I hope Git 3.0 is a good time to remove support for the current syntax and semantics. Then we can perhaps add the syntax back later with less surprising semantics.
Quoted 8 lines
Quoted 3 lines
Signed-off-by Martin von Zweigbergk <[email protected]>---
BreakingChanges: say that git diff X..Y syntax will be removed in 3.0I like it in prinicple and I do wish that we didn't do the lazy thing when we did the command line parser for "git diff" (we had revision range parser, so we just reused it instead of doing our own for "git diff"). But real life may bite us back.
Ah, so that's where it came from. Thanks for explaining. Speaking of revision range parsers, teaching Git something like Mercurial's or jj's "revsets" languages is one reason I would like to get rid of the git diff X..Y syntax here. I haven't done a comprehensive analysis but this is the only place I've noticed where we would need a breaking change if we ever wanted to teach Git revsets. (I'm not volunteering my time to work on such a project. I just think it would be nice if someone did :) )
Quoted 4 lines
In any case, a declaration that does not come with code changes that are protected by
WITH_BREAKING_CHANGESCPP macro is a patch that is not quite ready to be applied.
Yeah, this was meant as a discussion starter. I assumed I had missed a few things as I'm not very familiar with how things are done here. I'm happy to add that WITH_BREAKING_CHANGES macro if there's a V2.
Quoted 5 lines · Martin von Zweigbergk via GitGitGadget
From: Martin von Zweigbergk <[email protected]>
The
git diff X..Ysyntax is quite misleading because it looks like it shows the diff of the commits in the X..Y range but it actually shows the diff from X to Y.
Personally, I would like to see both the double-dot and triple-dot notations removed from the diff commands because they are often confused with the revision range notations. In my opinion, the double-dot notation doesn't even have much value as it can be replaced with:
A..B => A B
A.. => A @
..B => @ B
These alternatives are just as concise.
Quoted 5 lines
IMO, if that syntax is supported, it
should show a diff from the merge base of X and Y to Y. I hope Git 3.0 is a good time to remove support for the current syntax and semantics. Then we can perhaps add the syntax back later with less surprising semantics.
With the existing triple-dot notation, git diff A...B is equivalent to git diff $(git merge-base A B) B. I think this is what you are suggesting about that the double-dot notation should do. As mentioned earlier, I think both these notations are too easily confused with revision range notations so I think we should avoid using the dot syntax for such a shortcut altogether.
The triple-dot notation is a somewhat convienient shortcut though. If we wanted to remove it, we would maybe want to replace it some other functionally equivalent shortcut.
All this being said, I've sure there are folks in the wild using these notations in scripts and changing would cause disruption. Maybe the Git 3.0 release would indeed be a good time to remove them though.
-Justin
Quoted 11 lines · brian m. carlson
Documentation/BreakingChanges.adoc applications and forges. · line 119
Quoted 3 lines
Documentation/BreakingChanges.adoc +3
-0+Support for "git diff X..Y" syntax will be removed. Use "git diff X Y" instead. +This will open up the syntax for a more consistent interpretation of +"git diff $(git merge-base X Y) Y".I feel like this is going to break a whole lot of existing scripts and probably more than a few forges as well. It seems especially bad that we would add it back in the future with a completely different meaning, since we'll have some people that use 10-year LTS distros that go from, say, Git 2.51 to Git 3.xx, where the latter reintroduces the syntax with different semantics.
Quoted 4 lines
We've never really changed the meaning of things like revisions or revision-adjacent code in the past and I think those kinds of things we're pretty much stuck with forever. With that in mind, I don't think this is a good idea.
I do not think X..Y (or X...Y), if accepted by commands, would never change their meanings in the middle of the commands' lives. Teaching "git diff" to complain and barf on X..Y is a possibility, but to do the same for X...Y, we would need to come up with an alternative syntax first.
The same for "git checkout master..." that detaches HEAD at the fork point of the current topic (so that I can "git am" in a new iteration of patches on top). As the syntax "git diff master..." is symmetric with it, if one were to change, both should change to the same.
Thanks.
Quoted 11 lines · Justin Tobler
Quoted 5 lines · Martin von Zweigbergk via GitGitGadget
From: Martin von Zweigbergk <[email protected]>
The
git diff X..Ysyntax is quite misleading because it looks like it shows the diff of the commits in the X..Y range but it actually shows the diff from X to Y.Personally, I would like to see both the double-dot and triple-dot notations removed from the diff commands because they are often confused with the revision range notations.
Oh, I agree. I had forgotten that the triple-dot notation is also accepted.
Quoted 15 lines
In my opinion, the double-dot
notation doesn't even have much value as it can be replaced with:A..B => A B
A.. => A @
..B => @ BThese alternatives are just as concise.
Quoted 5 lines
IMO, if that syntax is supported, it
should show a diff from the merge base of X and Y to Y. I hope Git 3.0 is a good time to remove support for the current syntax and semantics. Then we can perhaps add the syntax back later with less surprising semantics.
Quoted 6 lines
With the existing triple-dot notation,
git diff A...Bis equivalent togit diff $(git merge-base A B) B. I think this is what you are suggesting about that the double-dot notation should do. As mentioned earlier, I think both these notations are too easily confused with revision range notations so I think we should avoid using the dot syntax for such a shortcut altogether.
FWIW, jj diff can diff between two commits with jj diff --from A --to B. It can also show the changes in commit A with jj diff -r A. You can also show the combined diffs in a range with jj diff -r A..B (i.e. jj diff -r A is a special case of that). I think that's consistent with the range notation because it's the same set of commits that are considered. But both git diff A..B and git diff A...B take range expressions and show diffs that don't correspond to those ranges. So I guess I'm saying that I'm not fundamentally opposed to having a way of showing the combined diff in a range, as long as it's consistent.
Quoted 4 lines
The triple-dot notation is a somewhat convienient shortcut though. If we wanted to remove it, we would maybe want to replace it some other functionally equivalent shortcut.
Makes sense. The problem is that there are not many symbols that are available without requiring shell escaping. I don't have a good suggestion.
Quoted 6 lines
All this being said, I've sure there are folks in the wild using these notations in scripts and changing would cause disruption. Maybe the Git 3.0 release would indeed be a good time to remove them though.
-Justin
Quoted 25 lines · Junio C Hamano
Quoted 11 lines · brian m. carlson
Documentation/BreakingChanges.adoc applications and forges. · line 119
Quoted 3 lines
Documentation/BreakingChanges.adoc +3
-0+Support for "git diff X..Y" syntax will be removed. Use "git diff X Y" instead. +This will open up the syntax for a more consistent interpretation of +"git diff $(git merge-base X Y) Y".I feel like this is going to break a whole lot of existing scripts and probably more than a few forges as well. It seems especially bad that we would add it back in the future with a completely different meaning, since we'll have some people that use 10-year LTS distros that go from, say, Git 2.51 to Git 3.xx, where the latter reintroduces the syntax with different semantics.
Quoted 4 lines
We've never really changed the meaning of things like revisions or revision-adjacent code in the past and I think those kinds of things we're pretty much stuck with forever. With that in mind, I don't think this is a good idea.
I do not think X..Y (or X...Y), if accepted by commands, would never change their meanings in the middle of the commands' lives. Teaching "git diff" to complain and barf on X..Y is a possibility, but to do the same for X...Y, we would need to come up with an alternative syntax first.
Quoted 3 lines
The same for "git checkout master..." that detaches HEAD at the fork point of the current topic (so that I can "git am" in a new iteration of patches on top).
I couldn't get this to work:
$ git checkout main... -- fatal: invalid reference: main...
But don't worry about it. I think your point about there being other commands that support the triple-dot syntax is still valid.
Quoted 3 lines
As the syntax "git diff master..."
is symmetric with it, if one were to change, both should change to the same.
Agreed. Some syntax for getting the merge base revision makes sense.
Quoted 4 lines
Thanks.
Quoted 2 lines · Junio C Hamano
I do not think X..Y (or X...Y), if accepted by commands, would never change their meanings in the middle of the commands' lives.
Sorry, double-negation bites again. Please drop "not" from "I do not think" when you are reading the above.
Quoted 11 lines
Teaching "git diff" to complain and barf on X..Y is a possibility, but to do the same for X...Y, we would need to come up with an alternative syntax first.
The same for "git checkout master..." that detaches HEAD at the fork point of the current topic (so that I can "git am" in a new iteration of patches on top). As the syntax "git diff master..." is symmetric with it, if one were to change, both should change to the same.
Thanks.
Quoted 11 lines · Martin von Zweigbergk
Quoted 5 lines · Junio C Hamano
[snip]
The same for "git checkout master..." that detaches HEAD at the fork point of the current topic (so that I can "git am" in a new iteration of patches on top).
I couldn't get this to work:
$ git checkout main... -- fatal: invalid reference: main...
git checkout X... works for me. Apparently it is this part of the
doc: “As a special case, you may use <rev-a>...<rev-b> [...]”
Quoted 8 lines
But don't worry about it. I think your point about there being other commands that support the triple-dot syntax is still valid.
Quoted 3 lines
As the syntax "git diff master..."
is symmetric with it, if one were to change, both should change to the same.[snip]
Quoted 9 lines · Kristoffer Haugsbakk
Quoted 7 lines · Martin von Zweigbergk
Quoted 5 lines · Junio C Hamano
[snip]
The same for "git checkout master..." that detaches HEAD at the fork point of the current topic (so that I can "git am" in a new iteration of patches on top).
Quoted 7 lines
Quoted 4 lines
I couldn't get this to work:
$ git checkout main... -- fatal: invalid reference: main...
git checkout X...works for me. Apparently it is this part of the
doc: “As a special case, you may use <rev-a>...<rev-b> [...]”
Oh, I think I know what the problem is. The reason I tried it was that I was curious how it would behave when there are multiple merge bases, so I had set up a repo like that. Then I got this:
``` $ git checkout main... error: pathspec 'main...' did not match any file(s) known to git $ git checkout main... -- fatal: invalid reference: main... ```
I didn't expect those messages to mean "the common ancestor is ambiguous" so I didn't think to try with an unambiguous common ancestor.
Quoted 9 lines
Quoted 9 lines
But don't worry about it. I think your point about there being other commands that support the triple-dot syntax is still valid.
Quoted 3 lines
As the syntax "git diff master..."
is symmetric with it, if one were to change, both should change to the same.[snip]
Quoted 10 lines · Martin von Zweigbergk
Quoted 5 lines · Junio C Hamano
[snip]
In any case, a declaration that does not come with code changes that are protected by
WITH_BREAKING_CHANGESCPP macro is a patch that is not quite ready to be applied.Yeah, this was meant as a discussion starter. I assumed I had missed a few things as I'm not very familiar with how things are done here. I'm happy to add that
WITH_BREAKING_CHANGESmacro if there's a V2.
Is one potential outcome just to deprecate the notations without slating them for removal right away? According to the current document it seems that only core.commentString=auto has been both deprecated and slated for removal during the same release cycle. It looks like everything else was deprecated for a good while before the Git 3.0 plan.
Maybe ref files → reftable as well although that doesn’t deprecate anything.
Quoted 25 lines · Martin von Zweigbergk
Quoted 14 lines · Junio C Hamano
"Martin von Zweigbergk via GitGitGadget" <[email protected]>
Quoted 10 lines · writes:
From: Martin von Zweigbergk <[email protected]>
The
git diff X..Ysyntax is quite misleading because it looks like it shows the diff of the commits in the X..Y range but it actually shows the diff from X to Y. IMO, if that syntax is supported, it should show a diff from the merge base of X and Y to Y. I hope Git 3.0 is a good time to remove support for the current syntax and semantics. Then we can perhaps add the syntax back later with less surprising semantics.Quoted 8 lines
Quoted 3 lines
Signed-off-by Martin von Zweigbergk <[email protected]>---
BreakingChanges: say that git diff X..Y syntax will be removed in 3.0I like it in prinicple and I do wish that we didn't do the lazy thing when we did the command line parser for "git diff" (we had revision range parser, so we just reused it instead of doing our own for "git diff"). But real life may bite us back.
Quoted 8 lines
Ah, so that's where it came from. Thanks for explaining. Speaking of revision range parsers, teaching Git something like Mercurial's or jj's "revsets" languages is one reason I would like to get rid of the
git diff X..Ysyntax here. I haven't done a comprehensive analysis but this is the only place I've noticed where we would need a breaking change if we ever wanted to teach Git revsets. (I'm not volunteering my time to work on such a project. I just think it would be nice if someone did :) )
Buried in my todo list is a goal to teach Git about JJ's "::" syntax :) Fortunately, I don't think that requires this particular change (which I'm otherwise in favor of).
Signature · 1 line
D. Ben Knoble
Quoted 19 lines · Junio C Hamano
Quoted 11 lines · brian m. carlson
Documentation/BreakingChanges.adoc applications and forges. · line 119
Quoted 3 lines
Documentation/BreakingChanges.adoc +3
-0+Support for "git diff X..Y" syntax will be removed. Use "git diff X Y" instead. +This will open up the syntax for a more consistent interpretation of +"git diff $(git merge-base X Y) Y".I feel like this is going to break a whole lot of existing scripts and probably more than a few forges as well. It seems especially bad that we would add it back in the future with a completely different meaning, since we'll have some people that use 10-year LTS distros that go from, say, Git 2.51 to Git 3.xx, where the latter reintroduces the syntax with different semantics.
Quoted 4 lines
We've never really changed the meaning of things like revisions or revision-adjacent code in the past and I think those kinds of things we're pretty much stuck with forever. With that in mind, I don't think this is a good idea.
Quoted 5 lines
I do not think X..Y (or X...Y), if accepted by commands, would never change their meanings in the middle of the commands' lives. Teaching "git diff" to complain and barf on X..Y is a possibility, but to do the same for X...Y, we would need to come up with an alternative syntax first.
Isn't the alternative syntax
git diff --merge-base X Y
? That's what the manual says, at any rate.
Quoted 5 lines
The same for "git checkout master..." that detaches HEAD at the fork point of the current topic (so that I can "git am" in a new iteration of patches on top). As the syntax "git diff master..." is symmetric with it, if one were to change, both should change to the same.
As a gut reaction, this is a bit apples-to-oranges: for me, the issue with the diff notations is that "git diff X...Y" shows changes "only on the Y side" (where as with rev-list/log/etc. it does "both sides"); contrast with "X..Y" in both scenarios.
Meanwhile, checkout is only ever really expecting a single point to checkout. Still, perhaps a different notation that means "merge-base" is warranted for that case, making the following equivalent with my hypothetical syntax:
git log X...Y
git log X Y X^{merge-Y}
git log X Y X^{M-Y} # hyphen optional here? "merge/M" vs "mergebase/MB"?
Inspiration from X^{/search}, of course, since any non-<type> and non-"/" prefix is effectively unused. Anyway, then you'd write
git checkout master^{M}or some such (where the "empty" bit becomes a synonym for HEAD as usual).
Signature · 1 line
D. Ben Knoble