Re: [PATCH v6 17/21] range-diff: populate the man page
Hi Peff,
On Mon, 10 Sep 2018, Jeff King wrote:
> On Sun, Sep 09, 2018 at 07:19:51PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> > >> And then I turn that into:
> > >>
> > >> # @{u} because I happen to be on 'master' and it's shorter to type
> > >> # than origin/master...
> > >> git range-diff @{u} 38b5f0fe72...718fbdedbc
> > >
> > > I don't understand what you want with that @{u} or 'origin/master' in
> > > the first place. It's unnecessary, the three-dot notation on its own
> > > works just fine.
> >
> > Maybe I've been using the wrong mode all along, I passed over by habits
> > from tbdiff, which were surely copy/pasted from somewhere.
> >
> > Looking at the git-range-diff manpage though it recommends
> > over ... when the topic has been rebased, which is
> > usually the case for e.g. a topic that's submitted to git.git (usually
> > be the time feedback has been gathered & a re-submission has been made
> > Junio has pushed another "master").
> >
> > So isn't " " the right thing to use over
> > "..." for git.git use? I think so, but I'm not sure.
>
> The problem with ... is that it finds the actual merge base,
> not the beginning of the topic.
That is actually not true, not for `range-diff`. If it sees `A...B`, it
will automatically generate `B..A A..B` from it.
That matters if the branches `A` and `B` have multiple merge bases.
> So if you have a 5-patch topic, but the first two patches weren't
> changed in the rebase, it won't show them at all! I made this mistake
> in [1], for example.
Yep, that is very easy to do.
Another thing to note is that often `A...B` is not doing the right thing
with branches that go into `pu` because some of us contributors rebase
to `master` (or `next`) between iterations. For such a use case, I
myself prefer the `@{u}` version that Ævar wants to use. (Although I
leave off the three dots, in which case everything works quite
magically.)
> For a force-push, though, you may not care about seeing the topic as a
> whole, and that mid-topic merge-base could be just fine. So pasting just
> the "A...B" works.
>
> I don't think your "@{u} A...B" makes any sense. You're giving _two_
> bases, which is weird. But even if you wanted to ignore the "..." base
> as a convenience to users of fetch, @{u} does not necessarily have
> anything to do with the @{upstream} of the topic at "A". You really want
> branch@{u}, which is on a separate part of the fetch output line (and
> your branch@{u} and the remote's are not necessarily the same, either;
> in this case you probably do not even have that branch checked out).
While `@{u}` in general does not relate to `A` nor `B`, it is quite
possible that it always does in Ævar's scenario. I would not want to
limit them in how they want to use Git from this point of view.
However, I would have a little bit of a problem with special-casing the
two-arg version when there are no dots in the first arg, and three dots
in the second one.
The problem here: the two-arg version already has a meaning: two commit
ranges. And it *is* conceivable that somebody wants to compare, say, the
full history of `git-gui.git` with a certain symmetric range in `pu`.
Granted, that is very obscure a use case, but it would be hard to
explain why the two-arg case refers to two commit ranges in some cases,
and in other cases not.
Ciao,
Dscho
>
> -Peff
>
> [1] https://public-inbox.org/git/20180821195102.gb...@sigill.intra.peff.net/
>
Re: [PATCH v6 17/21] range-diff: populate the man page
Ævar Arnfjörð Bjarmason writes: > Looking at the git-range-diff manpage though it recommends > over ... when the topic has been rebased, which is > usually the case for e.g. a topic that's submitted to git.git (usually > be the time feedback has been gathered & a re-submission has been made > Junio has pushed another "master"). > > So isn't " " the right thing to use over > "..." for git.git use? I think so, but I'm not sure. If is forked from different base than where was forked, thenwould give you more sensible range. And such an update is inevitable when must rely on new things that recently appeared on since forked from the mainline. But otherwise ... should work just fine. > In any case, there are going to be those use-case where you should be > using " ", and a rebase will be propagated by a > force-push, so I thought it made sense that range-diff could directly > consume the output of "fetch" in that case... I am not absolutely sure if there is *more* useful interpretation that " ..." may want to mean than to serve as a synonym for " " for those who are too lazy to type. But if there isn't, I'd say it is a reasonable synonym to want.
Re: [PATCH v6 17/21] range-diff: populate the man page
On Sun, Sep 09, 2018 at 07:19:51PM +0200, Ævar Arnfjörð Bjarmason wrote:
> >> And then I turn that into:
> >>
> >> # @{u} because I happen to be on 'master' and it's shorter to type
> >> # than origin/master...
> >> git range-diff @{u} 38b5f0fe72...718fbdedbc
> >
> > I don't understand what you want with that @{u} or 'origin/master' in
> > the first place. It's unnecessary, the three-dot notation on its own
> > works just fine.
>
> Maybe I've been using the wrong mode all along, I passed over by habits
> from tbdiff, which were surely copy/pasted from somewhere.
>
> Looking at the git-range-diff manpage though it recommends
> over ... when the topic has been rebased, which is
> usually the case for e.g. a topic that's submitted to git.git (usually
> be the time feedback has been gathered & a re-submission has been made
> Junio has pushed another "master").
>
> So isn't " " the right thing to use over
> "..." for git.git use? I think so, but I'm not sure.
The problem with ... is that it finds the actual merge base,
not the beginning of the topic. So if you have a 5-patch topic, but the
first two patches weren't changed in the rebase, it won't show them at
all! I made this mistake in [1], for example.
For a force-push, though, you may not care about seeing the topic as a
whole, and that mid-topic merge-base could be just fine. So pasting just
the "A...B" works.
I don't think your "@{u} A...B" makes any sense. You're giving _two_
bases, which is weird. But even if you wanted to ignore the "..." base
as a convenience to users of fetch, @{u} does not necessarily have
anything to do with the @{upstream} of the topic at "A". You really want
branch@{u}, which is on a separate part of the fetch output line (and
your branch@{u} and the remote's are not necessarily the same, either;
in this case you probably do not even have that branch checked out).
-Peff
[1] https://public-inbox.org/git/20180821195102.gb...@sigill.intra.peff.net/
Re: [PATCH v6 17/21] range-diff: populate the man page
On Sun, Sep 09 2018, SZEDER Gábor wrote:
> On Sun, Sep 09, 2018 at 01:14:25PM +0200, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Mon, Aug 13 2018, Johannes Schindelin via GitGitGadget wrote:
>>
>> I realize this topic has long since landed, just seemed like a good
>> thing to reply to to ask this question:
>>
>> > [...]
>> > + ( | ... |)
>> > [...]
>> > + ::
>> > + Compare the commits specified by the two ranges, where
>> > + `` is considered an older version of ``.
>> > +
>> > +...::
>> > + Equivalent to passing `..` and `..`.
>> > +
>> > + ::
>> > + Equivalent to passing `..` and `..`.
>> > + Note that `` does not need to be the exact branch point
>> > + of the branches. Example: after rebasing a branch `my-topic`,
>> > + `git range-diff my-topic@{u} my-topic@{1} my-topic` would
>> > + show the differences introduced by the rebase.
>>
>> I find myself using range-diff often by watching forced pushes to public
>> repos to see what others are doing, e.g. just now:
>>
>> + 38b5f0fe72...718fbdedbc split-index-racy ->
>> szeder/split-index-racy (forced update)
>
> Heh, spying on my wip bugfixes :)
>
>> And then I turn that into:
>>
>> # @{u} because I happen to be on 'master' and it's shorter to type
>> # than origin/master...
>> git range-diff @{u} 38b5f0fe72...718fbdedbc
>
> I don't understand what you want with that @{u} or 'origin/master' in
> the first place. It's unnecessary, the three-dot notation on its own
> works just fine.
Maybe I've been using the wrong mode all along, I passed over by habits
from tbdiff, which were surely copy/pasted from somewhere.
Looking at the git-range-diff manpage though it recommends
over ... when the topic has been rebased, which is
usually the case for e.g. a topic that's submitted to git.git (usually
be the time feedback has been gathered & a re-submission has been made
Junio has pushed another "master").
So isn't " " the right thing to use over
"..." for git.git use? I think so, but I'm not sure.
In any case, there are going to be those use-case where you should be
using " ", and a rebase will be propagated by a
force-push, so I thought it made sense that range-diff could directly
consume the output of "fetch" in that case...
>> Only to get an error because it doesn't support that, but just:
>>
>> git range-diff @{u} 38b5f0fe72 718fbdedbc
>>
>> I think it would be convenient given that "fetch" produces this output
>> to support this sort of invocation as synonymous with the three-arg
>> form. Then you can directly copy/paste that from terminals that have a
>> convenient feature to highlight a continuous \S+ reason to copy/paste
>> it.
>>
>> I can patch it in, but maybe there's UI reasons not to do this that I'm
>> missing, e.g. confusion with the existing ... syntax. What
>> do you think?
Re: [PATCH v6 17/21] range-diff: populate the man page
On Sun, Sep 09, 2018 at 01:14:25PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> On Mon, Aug 13 2018, Johannes Schindelin via GitGitGadget wrote:
>
> I realize this topic has long since landed, just seemed like a good
> thing to reply to to ask this question:
>
> > [...]
> > + ( | ... |)
> > [...]
> > + ::
> > + Compare the commits specified by the two ranges, where
> > + `` is considered an older version of ``.
> > +
> > +...::
> > + Equivalent to passing `..` and `..`.
> > +
> > + ::
> > + Equivalent to passing `..` and `..`.
> > + Note that `` does not need to be the exact branch point
> > + of the branches. Example: after rebasing a branch `my-topic`,
> > + `git range-diff my-topic@{u} my-topic@{1} my-topic` would
> > + show the differences introduced by the rebase.
>
> I find myself using range-diff often by watching forced pushes to public
> repos to see what others are doing, e.g. just now:
>
> + 38b5f0fe72...718fbdedbc split-index-racy ->
> szeder/split-index-racy (forced update)
Heh, spying on my wip bugfixes :)
> And then I turn that into:
>
> # @{u} because I happen to be on 'master' and it's shorter to type
> # than origin/master...
> git range-diff @{u} 38b5f0fe72...718fbdedbc
I don't understand what you want with that @{u} or 'origin/master' in
the first place. It's unnecessary, the three-dot notation on its own
works just fine.
> Only to get an error because it doesn't support that, but just:
>
> git range-diff @{u} 38b5f0fe72 718fbdedbc
>
> I think it would be convenient given that "fetch" produces this output
> to support this sort of invocation as synonymous with the three-arg
> form. Then you can directly copy/paste that from terminals that have a
> convenient feature to highlight a continuous \S+ reason to copy/paste
> it.
>
> I can patch it in, but maybe there's UI reasons not to do this that I'm
> missing, e.g. confusion with the existing ... syntax. What
> do you think?
Re: [PATCH v6 17/21] range-diff: populate the man page
On Mon, Aug 13 2018, Johannes Schindelin via GitGitGadget wrote:
I realize this topic has long since landed, just seemed like a good
thing to reply to to ask this question:
> [...]
> + ( | ... |)
> [...]
> + ::
> + Compare the commits specified by the two ranges, where
> + `` is considered an older version of ``.
> +
> +...::
> + Equivalent to passing `..` and `..`.
> +
> + ::
> + Equivalent to passing `..` and `..`.
> + Note that `` does not need to be the exact branch point
> + of the branches. Example: after rebasing a branch `my-topic`,
> + `git range-diff my-topic@{u} my-topic@{1} my-topic` would
> + show the differences introduced by the rebase.
I find myself using range-diff often by watching forced pushes to public
repos to see what others are doing, e.g. just now:
+ 38b5f0fe72...718fbdedbc split-index-racy ->
szeder/split-index-racy (forced update)
And then I turn that into:
# @{u} because I happen to be on 'master' and it's shorter to type
# than origin/master...
git range-diff @{u} 38b5f0fe72...718fbdedbc
Only to get an error because it doesn't support that, but just:
git range-diff @{u} 38b5f0fe72 718fbdedbc
I think it would be convenient given that "fetch" produces this output
to support this sort of invocation as synonymous with the three-arg
form. Then you can directly copy/paste that from terminals that have a
convenient feature to highlight a continuous \S+ reason to copy/paste
it.
I can patch it in, but maybe there's UI reasons not to do this that I'm
missing, e.g. confusion with the existing ... syntax. What
do you think?
