| Message ID | bc7c3f9d769b2d5a108ff4cdc3c7277e112fdb56.1552820745.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-diff.txt: prefer not using <commit>..<commit> | expand |
On Sun, Mar 17, 2019 at 6:09 PM Denton Liu <liu.denton@gmail.com> wrote: > > The documentation used to consider > > git diff <commit> <commit> > > and > > git diff <commit>..<commit> > > to be equal counterparts. However, rev-list-ish commands also use the > <commit>..<commit> notation, but in a logically conflicting manner which > was confusing for some users (including me!). > > Deprecating the notation entirely is not really an option because it > would be an arduous process without much end-value. In addition, there > are some valid use-cases that we don't want to break. > > Document the preference of the first form so that future confusion can > be minimised. > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > > Thanks all on your feedback on the discussion thread[1]! I opted for > just the documentation-only route so we don't break any valid use-cases. > > [1]: https://public-inbox.org/git/20190311093751.GA31092@archbookpro.localdomain/ > > Documentation/git-diff.txt | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt > index 72179d993c..10c7a2220c 100644 > --- a/Documentation/git-diff.txt > +++ b/Documentation/git-diff.txt > @@ -63,7 +63,11 @@ two blob objects, or changes between two files on disk. > > 'git diff' [<options>] <commit>..<commit> [--] [<path>...]:: > > - This is synonymous to the previous form. If <commit> on > + This is synonymous to the previous form. However, > + users should prefer the previous form over this form > + as this form may be more confusing due to the same > + notation having a logically conflicting meaning in > + linkgit:git-rev-list[1]-ish commands. If <commit> on > one side is omitted, it will have the same effect as > using HEAD instead. This is fine as-is. But another option to reduce even more exposure of these forms (both <commit>..[<commit>] and <commit>...[<commit>]) is to delete these forms in "DESCRIPTION" section and add maybe "EXOTIC SYNTAX" (or something) section after "OPTIONS" for just them. > -- > 2.21.0.512.g57bf1b23e1 >
On Sun, Mar 17 2019, Denton Liu wrote: > The documentation used to consider > > git diff <commit> <commit> > > and > > git diff <commit>..<commit> > > to be equal counterparts. However, rev-list-ish commands also use the > <commit>..<commit> notation, but in a logically conflicting manner which > was confusing for some users (including me!). > > Deprecating the notation entirely is not really an option because it > would be an arduous process without much end-value. In addition, there > are some valid use-cases that we don't want to break. > > Document the preference of the first form so that future confusion can > be minimised. > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > > Thanks all on your feedback on the discussion thread[1]! I opted for > just the documentation-only route so we don't break any valid use-cases. > > [1]: https://public-inbox.org/git/20190311093751.GA31092@archbookpro.localdomain/ > > Documentation/git-diff.txt | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt > index 72179d993c..10c7a2220c 100644 > --- a/Documentation/git-diff.txt > +++ b/Documentation/git-diff.txt > @@ -63,7 +63,11 @@ two blob objects, or changes between two files on disk. > > 'git diff' [<options>] <commit>..<commit> [--] [<path>...]:: > > - This is synonymous to the previous form. If <commit> on > + This is synonymous to the previous form. However, > + users should prefer the previous form over this form > + as this form may be more confusing due to the same > + notation having a logically conflicting meaning in > + linkgit:git-rev-list[1]-ish commands. If <commit> on > one side is omitted, it will have the same effect as > using HEAD instead. I think we're better off just consistently recommending "A..B" instead of "A B" and "fixing" any occurrence of the latter to the former. I.e. not taking this patch & going in the other direction. As noted in the thread you linked we'll always need ".." when one side is "HEAD" implicitly, and that's a really common case. So as confusing as the whole ".." v.s. "..." is in diff v.s. log I think we're worse off with "A B", since we'll *still* need to document the likes of "A.." and how that differs from log "A.." or "A...". So sometimes using the whitespace form for two revs and then the ".." when we just have one side makes things more confusing, not less. The reader will be left having to juggle more complexity in their head, not less.
On Sun, Mar 17, 2019 at 8:41 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > > On Sun, Mar 17 2019, Denton Liu wrote: > > > The documentation used to consider > > > > git diff <commit> <commit> > > > > and > > > > git diff <commit>..<commit> > > > > to be equal counterparts. However, rev-list-ish commands also use the > > <commit>..<commit> notation, but in a logically conflicting manner which > > was confusing for some users (including me!). > > > > Deprecating the notation entirely is not really an option because it > > would be an arduous process without much end-value. In addition, there > > are some valid use-cases that we don't want to break. > > > > Document the preference of the first form so that future confusion can > > be minimised. > > > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > > --- > > > > Thanks all on your feedback on the discussion thread[1]! I opted for > > just the documentation-only route so we don't break any valid use-cases. > > > > [1]: https://public-inbox.org/git/20190311093751.GA31092@archbookpro.localdomain/ > > > > Documentation/git-diff.txt | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt > > index 72179d993c..10c7a2220c 100644 > > --- a/Documentation/git-diff.txt > > +++ b/Documentation/git-diff.txt > > @@ -63,7 +63,11 @@ two blob objects, or changes between two files on disk. > > > > 'git diff' [<options>] <commit>..<commit> [--] [<path>...]:: > > > > - This is synonymous to the previous form. If <commit> on > > + This is synonymous to the previous form. However, > > + users should prefer the previous form over this form > > + as this form may be more confusing due to the same > > + notation having a logically conflicting meaning in > > + linkgit:git-rev-list[1]-ish commands. If <commit> on > > one side is omitted, it will have the same effect as > > using HEAD instead. > > I think we're better off just consistently recommending "A..B" instead > of "A B" and "fixing" any occurrence of the latter to the > former. I.e. not taking this patch & going in the other direction. > > As noted in the thread you linked we'll always need ".." when one side > is "HEAD" implicitly, and that's a really common case. You could just type " @" instead of "..". And that one is easier to explain. > So as confusing as the whole ".." v.s. "..." is in diff v.s. log I think > we're worse off with "A B", since we'll *still* need to document the > likes of "A.." and how that differs from log "A.." or "A...". > > So sometimes using the whitespace form for two revs and then the ".." > when we just have one side makes things more confusing, not less. The > reader will be left having to juggle more complexity in their head, not > less.
On Sun, Mar 17 2019, Duy Nguyen wrote: > On Sun, Mar 17, 2019 at 8:41 PM Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> >> >> On Sun, Mar 17 2019, Denton Liu wrote: >> >> > The documentation used to consider >> > >> > git diff <commit> <commit> >> > >> > and >> > >> > git diff <commit>..<commit> >> > >> > to be equal counterparts. However, rev-list-ish commands also use the >> > <commit>..<commit> notation, but in a logically conflicting manner which >> > was confusing for some users (including me!). >> > >> > Deprecating the notation entirely is not really an option because it >> > would be an arduous process without much end-value. In addition, there >> > are some valid use-cases that we don't want to break. >> > >> > Document the preference of the first form so that future confusion can >> > be minimised. >> > >> > Signed-off-by: Denton Liu <liu.denton@gmail.com> >> > --- >> > >> > Thanks all on your feedback on the discussion thread[1]! I opted for >> > just the documentation-only route so we don't break any valid use-cases. >> > >> > [1]: https://public-inbox.org/git/20190311093751.GA31092@archbookpro.localdomain/ >> > >> > Documentation/git-diff.txt | 6 +++++- >> > 1 file changed, 5 insertions(+), 1 deletion(-) >> > >> > diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt >> > index 72179d993c..10c7a2220c 100644 >> > --- a/Documentation/git-diff.txt >> > +++ b/Documentation/git-diff.txt >> > @@ -63,7 +63,11 @@ two blob objects, or changes between two files on disk. >> > >> > 'git diff' [<options>] <commit>..<commit> [--] [<path>...]:: >> > >> > - This is synonymous to the previous form. If <commit> on >> > + This is synonymous to the previous form. However, >> > + users should prefer the previous form over this form >> > + as this form may be more confusing due to the same >> > + notation having a logically conflicting meaning in >> > + linkgit:git-rev-list[1]-ish commands. If <commit> on >> > one side is omitted, it will have the same effect as >> > using HEAD instead. >> >> I think we're better off just consistently recommending "A..B" instead >> of "A B" and "fixing" any occurrence of the latter to the >> former. I.e. not taking this patch & going in the other direction. >> >> As noted in the thread you linked we'll always need ".." when one side >> is "HEAD" implicitly, and that's a really common case. > > You could just type " @" instead of "..". And that one is easier to explain. Sure, and if we're going to change our docs to consistently use @ at either side of such ranges instead of the empty string for "HEAD" I think that's worth discussing if the goal is to get rid of ".." for "diff". I'm commenting on the in-between state being more confusing to users, which is as far as this patch gets us. >> So as confusing as the whole ".." v.s. "..." is in diff v.s. log I think >> we're worse off with "A B", since we'll *still* need to document the >> likes of "A.." and how that differs from log "A.." or "A...". >> >> So sometimes using the whitespace form for two revs and then the ".." >> when we just have one side makes things more confusing, not less. The >> reader will be left having to juggle more complexity in their head, not >> less.
Duy Nguyen <pclouds@gmail.com> writes: >> - This is synonymous to the previous form. If <commit> on >> + This is synonymous to the previous form. However, >> + users should prefer the previous form over this form >> + as this form may be more confusing due to the same >> + notation having a logically conflicting meaning in >> + linkgit:git-rev-list[1]-ish commands. If <commit> on >> one side is omitted, it will have the same effect as >> using HEAD instead. > > This is fine as-is. But another option to reduce even more exposure of > these forms (both <commit>..[<commit>] and <commit>...[<commit>]) is > to delete these forms in "DESCRIPTION" section and add maybe "EXOTIC > SYNTAX" (or something) section after "OPTIONS" for just them. There is no other way to express A...B (well, short of spelling it out as "$(git merge-base A B) B"), so while it makes quite a lot of sense to discourage A..B (simply because .. is unnecessary and can be replace with a string with one fewer letter in it, namely " "), I am not sure if it is wise to throw the three-dot form into the same basket.
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Sure, and if we're going to change our docs to consistently use @ at > either side of such ranges instead of the empty string for "HEAD" I > think that's worth discussing if the goal is to get rid of ".." for > "diff". > > I'm commenting on the in-between state being more confusing to users, > which is as far as this patch gets us. Yuck. Let's not spread the @ disease. We are not Perl where being closer to line noise is somehow considered cool. Spell it out as HEAD, please.
On Sun, Mar 17, 2019 at 6:41 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Sun, Mar 17 2019, Denton Liu wrote: > > > diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt > > index 72179d993c..10c7a2220c 100644 > > --- a/Documentation/git-diff.txt > > +++ b/Documentation/git-diff.txt > > @@ -63,7 +63,11 @@ two blob objects, or changes between two files on disk. > > > > 'git diff' [<options>] <commit>..<commit> [--] [<path>...]:: > > > > - This is synonymous to the previous form. If <commit> on > > + This is synonymous to the previous form. However, > > + users should prefer the previous form over this form > > + as this form may be more confusing due to the same > > + notation having a logically conflicting meaning in > > + linkgit:git-rev-list[1]-ish commands. If <commit> on > > one side is omitted, it will have the same effect as > > using HEAD instead. > > I think we're better off just consistently recommending "A..B" instead > of "A B" and "fixing" any occurrence of the latter to the > former. I.e. not taking this patch & going in the other direction. > > As noted in the thread you linked we'll always need ".." when one side > is "HEAD" implicitly, and that's a really common case. By "really common" do you simply mean it is used by enough people that it should be supported, or are you trying to claim something about it's relative usage compared to "diff A B"? > So as confusing as the whole ".." v.s. "..." is in diff v.s. log I think > we're worse off with "A B", since we'll *still* need to document the > likes of "A.." and how that differs from log "A.." or "A...". > > So sometimes using the whitespace form for two revs and then the ".." > when we just have one side makes things more confusing, not less. The > reader will be left having to juggle more complexity in their head, not > less. Here I think you are implying that "A.." or "..A" is somehow on a similar magnitude of usage as "A B". Is that accurate? If that reflected real-world usage, I would be more inclined to agree with your course of action, but I can't fathom them having similar usage rates. Personally, I don't think I've ever seen any user use "A.." or "..A" (well, except me), and only rarely even use "A..B"; with users I have worked with and supported and taught, I'd guess "diff A B" is used far more (at least an order of magnitude more) than any other form, and that only some of them and only occasionally want to use anything else, such as diffing against a merge base. This whole topic came up again, IMO, because for the occasional usecase of diffing against a merge-base, people's intuition leads them to '..' instead of '...', and we wanted to fix or help avoid that problem. To me, your solution sounds like you want to instead embrace the confusion: force people to deal with it early and often so that they become trained on it and can articulate the differences between '..' and '...' for both diff and log. Is that accurate, or am I misunderstanding/mis-stating your strategy?
Hi Denton, Thanks for working on this. Some thoughts... On Sun, Mar 17, 2019 at 4:09 AM Denton Liu <liu.denton@gmail.com> wrote: > > The documentation used to consider > > git diff <commit> <commit> > > and > > git diff <commit>..<commit> > > to be equal counterparts. However, rev-list-ish commands also use the > <commit>..<commit> notation, but in a logically conflicting manner which > was confusing for some users (including me!). > > Deprecating the notation entirely is not really an option because it > would be an arduous process without much end-value. In addition, there > are some valid use-cases that we don't want to break. Yes, there were multiple people who commented that they liked to copy-paste the "A..B" output from fetch/pull in combination with diff and log (even though one suggested that this gave the wrong output and what they really wanted was "diff A...B"). However, "removal of functionality" isn't the only form of deprecation/warning. Updating the manpage is another one which you implemented, but I'd like to suggest yet another: Prefix the diff with a warning message, e.g. "WARNING: You ran 'git diff A..B' (which means the same thing as 'git diff A B'). Many users confuse 'git diff A..B' and 'git diff A...B'. Please see 'git diff --help' for more details." Having extra text (e.g. commit message or warning) at the beginning of the diff does not prevent tools like patch(1) or git-apply(1) from successfully applying it, it still makes sense to humans (and who as an added bonus happen to be really good at filtering out common messages if they do encounter them more than a few times), and gives us a chance in the future to figure out how to potentially extend the message to make it a deprecation warning and/or provide details about how to change the behavior of '..' to either be an error or behave like triple dots or just not warn. Elijah
On Mon, Mar 18 2019, Elijah Newren wrote: > On Sun, Mar 17, 2019 at 6:41 AM Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> On Sun, Mar 17 2019, Denton Liu wrote: >> >> > diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt >> > index 72179d993c..10c7a2220c 100644 >> > --- a/Documentation/git-diff.txt >> > +++ b/Documentation/git-diff.txt >> > @@ -63,7 +63,11 @@ two blob objects, or changes between two files on disk. >> > >> > 'git diff' [<options>] <commit>..<commit> [--] [<path>...]:: >> > >> > - This is synonymous to the previous form. If <commit> on >> > + This is synonymous to the previous form. However, >> > + users should prefer the previous form over this form >> > + as this form may be more confusing due to the same >> > + notation having a logically conflicting meaning in >> > + linkgit:git-rev-list[1]-ish commands. If <commit> on >> > one side is omitted, it will have the same effect as >> > using HEAD instead. >> >> I think we're better off just consistently recommending "A..B" instead >> of "A B" and "fixing" any occurrence of the latter to the >> former. I.e. not taking this patch & going in the other direction. >> >> As noted in the thread you linked we'll always need ".." when one side >> is "HEAD" implicitly, and that's a really common case. > > By "really common" do you simply mean it is used by enough people that > it should be supported, or are you trying to claim something about > it's relative usage compared to "diff A B"? > >> So as confusing as the whole ".." v.s. "..." is in diff v.s. log I think >> we're worse off with "A B", since we'll *still* need to document the >> likes of "A.." and how that differs from log "A.." or "A...". >> >> So sometimes using the whitespace form for two revs and then the ".." >> when we just have one side makes things more confusing, not less. The >> reader will be left having to juggle more complexity in their head, not >> less. > > Here I think you are implying that "A.." or "..A" is somehow on a > similar magnitude of usage as "A B". Is that accurate? If that > reflected real-world usage, I would be more inclined to agree with > your course of action, but I can't fathom them having similar usage > rates. Personally, I don't think I've ever seen any user use "A.." or > "..A" (well, except me), and only rarely even use "A..B"; with users I > have worked with and supported and taught, I'd guess "diff A B" is > used far more (at least an order of magnitude more) than any other > form, and that only some of them and only occasionally want to use > anything else, such as diffing against a merge base. > > This whole topic came up again, IMO, because for the occasional > usecase of diffing against a merge-base, people's intuition leads them > to '..' instead of '...', and we wanted to fix or help avoid that > problem. To me, your solution sounds like you want to instead embrace > the confusion: force people to deal with it early and often so that > they become trained on it and can articulate the differences between > '..' and '...' for both diff and log. Is that accurate, or am I > misunderstanding/mis-stating your strategy? Some of this thread's confusing, and on re-reading I see my reply hasn't helped much. To clarify. There's at least these things to consider: 1. What should the semantics of .. or ... be? 2. What semantics (regardless of syntax) should we recommend for common cases? 3. Depending on #1 and #2, can we make our docs less confusing? My opinion: 1. I'd ideally like to switch the semantics of ".." and "...". I don't think anyone argues that it would be a bad thing in theory if we'd started out that way, Whether it's worth switching now is another matter. Junio chimed in in <xmqqmum0h88n.fsf@gitster-ct.c.googlers.com> saying he's not for it, so I'm assuming it's out for now. 2. I agree that we should generally recommend what's now "..." instead of "..", whatever the syntax is to invoke that. 3. And now, coming to the point I was trying to make. Assuming we'd like to go for #2 as I noted, and switching them around as in #1 is a no-go, what syntax should we prefer for "X..Z", aka. "X Y"? I think to reduce doc confusion we'd be better of using ".." consistently over the whitespace form, since we're going to need to show diffs with HEAD on one side. Using either "@" or "HEAD" there is more verbose. So yeah, I guess I am in a narrow sense advocating for embracing the confusion. I'd rather not have it in the first place, but since we seem stuck with it let's at least stick with and recommend *one* syntax for that confusion, and pick the one we can consistently use for 100% of our examples. Anecdotally I've seen ".." in all forms be *way* more common among users, even though if you sat them down and explained to them what it does v.s. "..." they'd usually say they wanted the latter. I did some brief scraping of .bash_history on one of our big shared servers just now that has a lot of users (100-200) using git for daily development. I've got many pages of things like "@{u}..", "<tag1>..<tag2>", "<rev>.. -- <path>" etc. against just a couple of uses of "...". The "<rev1> <rev2>" form gets a bit of use, but maybe 1/2 of "..".
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Some of this thread's confusing, and on re-reading I see my reply hasn't > helped much. > > To clarify. There's at least these things to consider: > > 1. What should the semantics of .. or ... be? > 2. What semantics (regardless of syntax) should we recommend for common cases? > 3. Depending on #1 and #2, can we make our docs less confusing? Nice way to summarise. > My opinion: > > 1. I'd ideally like to switch the semantics of ".." and "...". I don't > think anyone argues that it would be a bad thing in theory if we'd > started out that way, Whether it's worth switching now is another > matter. > > Junio chimed in in <xmqqmum0h88n.fsf@gitster-ct.c.googlers.com> > saying he's not for it, so I'm assuming it's out for now. I do agree with the "in the ideal world, where we didn't have any existing users and had perfect foresight back when invention of three-dot form for 'log' was even a few months away down the road, we would have used diff A...B for the two-endpoints", and I also agree with "that is 10 years or more too late now---don't waste time even pondering about it". > > 2. I agree that we should generally recommend what's now "..." instead > of "..", whatever the syntax is to invoke that. I do not agree with this at all. When I want to compare what is in devel and what is in master, I may be interested in two completely different things depending on why I am doing the comparison. - I may want to know "what is in the development track that is yet to be merged to 'master'". - Or I may be looking for "what is the difference between the master branch and the development branch." The latter should match the former if 'devel' and 'master' means what we generally think they are, and by looking at the latter, I can find what would be missed if I wholesale replace 'master' with 'devel' before merging 'master' backwards into 'devel' to make the latter catch up. So even in that context, I would prefer to see the latter (needless to say, the start-up cost is far less for the latter than for the former). When we discuss a more generic "two endpoints comparison" whose relationship between the two endpoints is not as well-defined as 'devel' and 'master' (i.e. what does it mean to compare v1.2.1 and v1.3.4?), I can phrase in human-understandable terms what ".." does, but not what "..." does. I have no idea how you or anybody can say that "we should generally recommend" that the former is the norm and two-endpoint comparison is an exception.
On Mon, Mar 18, 2019 at 03:45:25PM +0900, Junio C Hamano wrote: > Duy Nguyen <pclouds@gmail.com> writes: > > >> - This is synonymous to the previous form. If <commit> on > >> + This is synonymous to the previous form. However, > >> + users should prefer the previous form over this form > >> + as this form may be more confusing due to the same > >> + notation having a logically conflicting meaning in > >> + linkgit:git-rev-list[1]-ish commands. If <commit> on > >> one side is omitted, it will have the same effect as > >> using HEAD instead. > > > > This is fine as-is. But another option to reduce even more exposure of > > these forms (both <commit>..[<commit>] and <commit>...[<commit>]) is > > to delete these forms in "DESCRIPTION" section and add maybe "EXOTIC > > SYNTAX" (or something) section after "OPTIONS" for just them. > > There is no other way to express A...B (well, short of spelling it > out as "$(git merge-base A B) B"), so while it makes quite a lot of > sense to discourage A..B (simply because .. is unnecessary and can > be replace with a string with one fewer letter in it, namely " "), > I am not sure if it is wise to throw the three-dot form into the > same basket. > Perhaps we could add an option to use the base as a comparison, something like "git diff --compare-base A B" which would mean "git diff $(git merge-base A B) B". If we do this, we could keep feature-parity while deprecating the confusing "range-notation" for diff. Then, we could possibly move .. and ... into a "RANGE-BASED SYNTAX" section and call it a day. I now personally don't think that deprecating it in the code (or even adding a warning) would be worth the effort since the syntax is so ingrained in common usage. However, I believe that we should encourage new users to avoid the range-syntax if possible.
On Mon, Mar 18, 2019 at 12:07 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Mon, Mar 18 2019, Elijah Newren wrote: > > > On Sun, Mar 17, 2019 at 6:41 AM Ævar Arnfjörð Bjarmason > > <avarab@gmail.com> wrote: > >> On Sun, Mar 17 2019, Denton Liu wrote: > >> > Some of this thread's confusing, and on re-reading I see my reply hasn't > helped much. > > To clarify. There's at least these things to consider: > > 1. What should the semantics of .. or ... be? > 2. What semantics (regardless of syntax) should we recommend for common cases? > 3. Depending on #1 and #2, can we make our docs less confusing? That's a good framework; thanks for clarifying. Junio has weighed in already, but there were two other things I thought I'd still comment on. First, I think this list could use another item, related to #3: 4. Given backward compatibility and existing use-case constraints, is there at least a way to warn users of possible confusion besides just the docs? I suggested one answer elsewhere in this thread (cf. CABPp-BEy9nN=aV8Y+ueYqv299umHoF2E=8D7heJARM4Qa7P5JQ@mail.gmail.com); not sure what others think of it yet. > Anecdotally I've seen ".." in all forms be *way* more common among > users, even though if you sat them down and explained to them what it > does v.s. "..." they'd usually say they wanted the latter. > > I did some brief scraping of .bash_history on one of our big shared > servers just now that has a lot of users (100-200) using git for daily > development. I've got many pages of things like "@{u}..", > "<tag1>..<tag2>", "<rev>.. -- <path>" etc. against just a couple of uses > of "...". The "<rev1> <rev2>" form gets a bit of use, but maybe 1/2 of > "..". Wow, that is extremely surprising to me. I have no mechanism to access any such data (nor any mechanism for pushing new git versions out to users other than sending out an email about new upstream releases and asking them to update). So, I did the next best thing and sent out a poll to developers (all of whom use git daily); there's a self-selection bias in the responses that probably tilts away from novice users, but it's all I had: Which do you use the most? 1.`git diff A B` `11` 2. `git diff A..B` `2` 3. `git diff A...B` 4. I never use any of these `5` The responses were 11 for #1 (with a number expressing in follow-ups that it was the only form of these three they ever used, though some still used other forms sometimes), 2 for #2 (one of whom responded to requests for more details and found out he was using it wrong and that he could save a character by using form #1 in cases where A was the merge base, which he was happy about), 0 for #3, and 5 for #4. That was still a lot more voting for the '..' form than I expected, but still far less than what you have seen. I wonder if there's a regional difference?
Hi Elijah, Sorry for the late reply, I've been mulling over it for the past couple of days. On Mon, Mar 18, 2019 at 10:59:56AM -0700, Elijah Newren wrote: > Hi Denton, > > Thanks for working on this. Some thoughts... > > On Sun, Mar 17, 2019 at 4:09 AM Denton Liu <liu.denton@gmail.com> wrote: > > > > The documentation used to consider > > > > git diff <commit> <commit> > > > > and > > > > git diff <commit>..<commit> > > > > to be equal counterparts. However, rev-list-ish commands also use the > > <commit>..<commit> notation, but in a logically conflicting manner which > > was confusing for some users (including me!). > > > > Deprecating the notation entirely is not really an option because it > > would be an arduous process without much end-value. In addition, there > > are some valid use-cases that we don't want to break. > > Yes, there were multiple people who commented that they liked to > copy-paste the "A..B" output from fetch/pull in combination with diff > and log (even though one suggested that this gave the wrong output and > what they really wanted was "diff A...B"). > > However, "removal of functionality" isn't the only form of > deprecation/warning. Updating the manpage is another one which you > implemented, but I'd like to suggest yet another: Prefix the diff with > a warning message, e.g. > > "WARNING: You ran 'git diff A..B' (which means the same thing as 'git > diff A B'). Many users confuse 'git diff A..B' and 'git diff A...B'. > Please see 'git diff --help' for more details." > > Having extra text (e.g. commit message or warning) at the beginning of > the diff does not prevent tools like patch(1) or git-apply(1) from > successfully applying it, it still makes sense to humans (and who as > an added bonus happen to be really good at filtering out common > messages if they do encounter them more than a few times), and gives > us a chance in the future to figure out how to potentially extend the > message to make it a deprecation warning and/or provide details about > how to change the behavior of '..' to either be an error or behave > like triple dots or just not warn. > > > Elijah I was originally planning on doing something like this (by warning to stderr, but your idea actually gets to the user ;) ). The only thing I'm worried about for a change like this is that it'll be very controversial since, as we could see anecdotally, a lot of people have trained themselves to use the .. form. In terms of backwards compatibility, I think that there's the possibility of breaking some scripts by doing this change but we could mitigate this by detecting if we're outputting to a terminal and only print the message in that case. Thanks for the suggestion! Denton
Hi Denton, On Wed, Mar 20, 2019 at 10:28 AM Denton Liu <liu.denton@gmail.com> wrote: > > Hi Elijah, > > Sorry for the late reply, I've been mulling over it for the past couple > of days. No worries; thanks for thinking about it. > On Mon, Mar 18, 2019 at 10:59:56AM -0700, Elijah Newren wrote: > > On Sun, Mar 17, 2019 at 4:09 AM Denton Liu <liu.denton@gmail.com> wrote: > > > > > > The documentation used to consider > > > > > > git diff <commit> <commit> > > > > > > and > > > > > > git diff <commit>..<commit> > > > > > > to be equal counterparts. However, rev-list-ish commands also use the > > > <commit>..<commit> notation, but in a logically conflicting manner which > > > was confusing for some users (including me!). > > > > > > Deprecating the notation entirely is not really an option because it > > > would be an arduous process without much end-value. In addition, there > > > are some valid use-cases that we don't want to break. > > > > Yes, there were multiple people who commented that they liked to > > copy-paste the "A..B" output from fetch/pull in combination with diff > > and log (even though one suggested that this gave the wrong output and > > what they really wanted was "diff A...B"). > > > > However, "removal of functionality" isn't the only form of > > deprecation/warning. Updating the manpage is another one which you > > implemented, but I'd like to suggest yet another: Prefix the diff with > > a warning message, e.g. > > > > "WARNING: You ran 'git diff A..B' (which means the same thing as 'git > > diff A B'). Many users confuse 'git diff A..B' and 'git diff A...B'. > > Please see 'git diff --help' for more details." > > > > Having extra text (e.g. commit message or warning) at the beginning of > > the diff does not prevent tools like patch(1) or git-apply(1) from > > successfully applying it, it still makes sense to humans (and who as > > an added bonus happen to be really good at filtering out common > > messages if they do encounter them more than a few times), and gives > > us a chance in the future to figure out how to potentially extend the > > message to make it a deprecation warning and/or provide details about > > how to change the behavior of '..' to either be an error or behave > > like triple dots or just not warn. > > I was originally planning on doing something like this (by warning to > stderr, but your idea actually gets to the user ;) ). The only thing I'm > worried about for a change like this is that it'll be very controversial > since, as we could see anecdotally, a lot of people have trained > themselves to use the .. form. > > In terms of backwards compatibility, I think that there's the > possibility of breaking some scripts by doing this change but we could > mitigate this by detecting if we're outputting to a terminal and only > print the message in that case. I don't see why anything would break if we just always showed the warning. git diff's purpose is to create a "patch", and as such can be used by the patch(1) command. We added additional headers in 'git diff' relative to diff(1), but it's still understandable by patch. We also created git-apply because it could potentially make use of the extra headers to do slightly smarter stuff than patch. But both allow arbitrary extra initial text as long as it doesn't have the relevant start-of-next-diff-hunk characters. Extra text at the beginning (an explanation or a commit message or a warning or whatever) thus doesn't change it from being a valid patch. Human beings will see a little extra text by default, but they already usually skip right over the diff headers when reading; this is essentially just another diff header. And we could provide them with a config option to turn it off if they used it all the time and found the two extra lines of text annoying. Since both Ævar and I noted that even among people who use '..' lots of them use it erroneously, it seems like a worthwhile warning to add, to me at least. That is, unless I've misunderstood my purpose of 'git-diff' as creating a patch or missed something else important. Anyway, just my $0.02. (And I'd be happy to take this on later if you don't want to). Elijah
Hi Junio, On Mon, 18 Mar 2019, Junio C Hamano wrote: > Duy Nguyen <pclouds@gmail.com> writes: > > >> - This is synonymous to the previous form. If <commit> on > >> + This is synonymous to the previous form. However, > >> + users should prefer the previous form over this form > >> + as this form may be more confusing due to the same > >> + notation having a logically conflicting meaning in > >> + linkgit:git-rev-list[1]-ish commands. If <commit> on > >> one side is omitted, it will have the same effect as > >> using HEAD instead. > > > > This is fine as-is. But another option to reduce even more exposure of > > these forms (both <commit>..[<commit>] and <commit>...[<commit>]) is > > to delete these forms in "DESCRIPTION" section and add maybe "EXOTIC > > SYNTAX" (or something) section after "OPTIONS" for just them. > > There is no other way to express A...B (well, short of spelling it > out as "$(git merge-base A B) B"), so while it makes quite a lot of > sense to discourage A..B (simply because .. is unnecessary and can > be replace with a string with one fewer letter in it, namely " "), > I am not sure if it is wise to throw the three-dot form into the > same basket. I just happened to come upon a use case where the `A..B` way is not actually useless: just after fetching a branch, I got the usual update lines, and it is pretty convenient to be able to use them not only for `git log`, but also for `git diff` invocations, to see what actually changed (from two angles). So maybe you could register this as a vote to not totally strike the A..B notation from the manual. Ciao, Dscho
diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt index 72179d993c..10c7a2220c 100644 --- a/Documentation/git-diff.txt +++ b/Documentation/git-diff.txt @@ -63,7 +63,11 @@ two blob objects, or changes between two files on disk. 'git diff' [<options>] <commit>..<commit> [--] [<path>...]:: - This is synonymous to the previous form. If <commit> on + This is synonymous to the previous form. However, + users should prefer the previous form over this form + as this form may be more confusing due to the same + notation having a logically conflicting meaning in + linkgit:git-rev-list[1]-ish commands. If <commit> on one side is omitted, it will have the same effect as using HEAD instead.
The documentation used to consider git diff <commit> <commit> and git diff <commit>..<commit> to be equal counterparts. However, rev-list-ish commands also use the <commit>..<commit> notation, but in a logically conflicting manner which was confusing for some users (including me!). Deprecating the notation entirely is not really an option because it would be an arduous process without much end-value. In addition, there are some valid use-cases that we don't want to break. Document the preference of the first form so that future confusion can be minimised. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- Thanks all on your feedback on the discussion thread[1]! I opted for just the documentation-only route so we don't break any valid use-cases. [1]: https://public-inbox.org/git/20190311093751.GA31092@archbookpro.localdomain/ Documentation/git-diff.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)