BreakingChanges: say that git diff X..Y syntax will be removed in 3.0
#1989
+4
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Welcome to GitGitGadgetHi @martinvonz, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests. Please make sure that either:
You can CC potential reviewers by adding a footer to the PR description with the following syntax: NOTE: DO NOT copy/paste your CC list from a previous GGG PR's description, Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:
It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code. Contributing the patchesBefore you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form Both the person who commented An alternative is the channel Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment If you want to see what email(s) would be sent for a After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail). If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the curl -g --user "<EMailAddress>:<Password>" \
--url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txtTo iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description): To send a new iteration, just add another PR comment with the contents: Need help?New contributors who want advice are encouraged to join git-mentoring@googlegroups.com, where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join. You may also be able to find help in real time in the developer IRC channel, |
|
There are issues in commit 6f0af37: |
martinvonz
force-pushed
the
mz/wtmnpolouvvz
branch
from
6f0af37 to
02c4934
Compare
|
/allow |
|
User martinvonz is now allowed to use GitGitGadget. WARNING: martinvonz has no public email address set on GitHub; GitGitGadget needs an email address to Cc: you on your contribution, so that you receive any feedback on the Git mailing list. Go to https://github.com/settings/profile to make your preferred email public to let GitGitGadget know which email address to use. |
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. Signed-off-by: Martin von Zweigbergk <martinvonz@google.com>
martinvonz
force-pushed
the
mz/wtmnpolouvvz
branch
from
02c4934 to
daedd97
Compare
|
/submit |
|
Submitted as pull.1989.git.1760566054455.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "Martin von Zweigbergk via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Martin von Zweigbergk <martinvonz@google.com>
>
> 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.
>
> Signed-off-by: Martin von Zweigbergk <martinvonz@google.com>
> ---
> 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.
> 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(+)
>
> diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc
> index 90b53abcea..93fb968840 100644
> --- a/Documentation/BreakingChanges.adoc
> +++ b/Documentation/BreakingChanges.adoc
> @@ -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. <2f5de416-04ba-c23d-1e0b-83bb655829a7@zombino.com>,
> <20170223155046.e7nxivfwqqoprsqj@LykOS.localdomain>,
> <CA+EOSBncr=4a4d8n9xS4FNehyebpmX8JiUwCsXD47EQDE+DiUQ@mail.gmail.com>.
>
> base-commit: 143f58ef7535f8f8a80d810768a18bdf3807de26 |
|
On the Git mailing list, "brian m. carlson" wrote (reply to this): On 2025-10-15 at 22:07:34, Martin von Zweigbergk via GitGitGadget wrote:
> From: Martin von Zweigbergk <martinvonz@google.com>
>
> 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.
>
> Signed-off-by: Martin von Zweigbergk <martinvonz@google.com>
> ---
> 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(+)
>
> diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc
> index 90b53abcea..93fb968840 100644
> --- a/Documentation/BreakingChanges.adoc
> +++ b/Documentation/BreakingChanges.adoc
> @@ -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.
--
brian m. carlson (they/them)
Toronto, Ontario, CA |
|
User |
|
On the Git mailing list, Martin von Zweigbergk wrote (reply to this): On Wed, 15 Oct 2025 at 15:19, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Martin von Zweigbergk via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Martin von Zweigbergk <martinvonz@google.com>
> >
> > 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.
> >
> > Signed-off-by: Martin von Zweigbergk <martinvonz@google.com>
> > ---
> > 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.
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 :) )
>
> 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.
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. |
|
User |
|
On the Git mailing list, Justin Tobler wrote (reply to this): On 25/10/15 10:07PM, Martin von Zweigbergk via GitGitGadget wrote:
> From: Martin von Zweigbergk <martinvonz@google.com>
>
> 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.
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.
> 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 |
|
User |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>> +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.
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.
|
|
On the Git mailing list, Martin von Zweigbergk wrote (reply to this): On Wed, 15 Oct 2025 at 18:28, Justin Tobler <jltobler@gmail.com> wrote:
>
> On 25/10/15 10:07PM, Martin von Zweigbergk via GitGitGadget wrote:
> > From: Martin von Zweigbergk <martinvonz@google.com>
> >
> > 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.
>
> 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.
> 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.
>
> > 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.
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.
>
> 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.
>
> 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 |
|
On the Git mailing list, Martin von Zweigbergk wrote (reply to this): On Thu, 16 Oct 2025 at 06:44, Junio C Hamano <gitster@pobox.com> wrote:
>
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> >> +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.
>
> 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).
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.
> 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.
>
> Thanks.
>
> |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Junio C Hamano <gitster@pobox.com> writes:
> 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.
> 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. |
|
On the Git mailing list, "Kristoffer Haugsbakk" wrote (reply to this): On Thu, Oct 16, 2025, at 18:38, Martin von Zweigbergk wrote:
> On Thu, 16 Oct 2025 at 06:44, Junio C Hamano <gitster@pobox.com> wrote:
>>[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> [...]”
>
> But don't worry about it. I think your point about there being other
> commands that support the triple-dot syntax is still valid.
>
>> As the syntax "git diff master..."
>> is symmetric with it, if one were to change, both should change to
>> the same.
>[snip] |
|
User |
|
On the Git mailing list, Martin von Zweigbergk wrote (reply to this): On Thu, 16 Oct 2025 at 10:02, Kristoffer Haugsbakk
<kristofferhaugsbakk@fastmail.com> wrote:
>
> On Thu, Oct 16, 2025, at 18:38, Martin von Zweigbergk wrote:
> > On Thu, 16 Oct 2025 at 06:44, Junio C Hamano <gitster@pobox.com> wrote:
> >>[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> [...]”
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.
>
> >
> > But don't worry about it. I think your point about there being other
> > commands that support the triple-dot syntax is still valid.
> >
> >> As the syntax "git diff master..."
> >> is symmetric with it, if one were to change, both should change to
> >> the same.
> >[snip] |
|
On the Git mailing list, "Kristoffer Haugsbakk" wrote (reply to this): On Thu, Oct 16, 2025, at 01:06, Martin von Zweigbergk wrote:
> On Wed, 15 Oct 2025 at 15:19, Junio C Hamano <gitster@pobox.com> wrote:
>>[snip]
>>
>> 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.
>
> 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.
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cc: "brian m. carlson" sandals@crustytoothpaste.net
cc: Martin von Zweigbergk martinvonz@google.com
cc: Justin Tobler jltobler@gmail.com
cc: "Kristoffer Haugsbakk" kristofferhaugsbakk@fastmail.com