Re: [PATCH 1/3] revlist.c: introduce --cherry for unsymmetric picking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> writes:

> The existing "--cherry-pick" does not work with unsymmetric ranges
> (A..B) for obvious reasons.
>
> Introduce "--cherry" which works more like "git-cherry", i.e.: Ignore
> commits in B which are patch-equivalent to patches in A, i.e. list only
> those which "git cherry B A" would list with a "-". This is especially
> useful for things like
>
> git log --cherry @{u}..
>
> which is a much more descriptive than
>
> git cherry @{u}
>
> and potentially more useful than
>
> git log --cherry-pick @{u}...
>
> Signed-off-by: Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx>

A few comments.

As a Porcelain tool, "log --cherry @{u}.." may very well be useful; what
do I have that I still need to send.  It is the moral equivalent of what
format-patch does internally.

However I don't find it "works more like 'git-cherry'" at all.  The point
of the command is to show _both_ what are still left to be sent, and what
are already accepted.  So it is very much inherently a symmetric-range
operation.  At the plumbing level (which 'git-cherry' was designed to be),
we should be able to ask for either (or both).  Adding a "we are only
interested in the right hand side" as a rev-list option is somewhat
distasteful and short-sighted.

I wonder if you can instead extend the revision walking machinery by just
adding a filter that says "show me only the left/right hand side" [*1*]?
I.e.

    git rev-list --right-only --oneline --cherry-pick @{u}...

If that is done, we could ask for "--left-only" when we wanted to.

As there are by definition more contributors than integrators, naturally
we can expect that "--right-only" would be a lot more often used mode of
operation than "--left-only", and I don't mind to have "--cherry" as a
synonym to trigger "--cherry-pick --right-only" (and perhaps internally
turn a two-dot automatically into three-dot), so that you can say

    git log --cherry @{u}..

from the command line to get the same effect.

By the way, when this feature is properly implemented internally at the
revision traversal level, we should be able to lose quite a lot of code
from builtin/log.c, as format-patch in it was the original implementation
of the whole thing, and it was done outside the revision walking machinery
to implement patch equivalence filtering of the traversal result.  We
would essentially feed the symmetric upstream...HEAD range with the
cherry-pick flag, ask it to give only the right hand side (i.e. what are
left after the patch equivalence filter), and emit the result.

The get_patch_ids() function itself can go, and its caller and the
codepaths that use has_commit_patch_id() would be vastly simplified, no?


[Footnote]

*1* It might be even interesting to add a mode that allows you to ask for
only the matching ones, but that would take somewhat a different logic in
cherry_pick_list() in revision.c.  Instead of filtering out the equivalent
ones, you would need to keep them, painted in yet another color.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]