Re: [PATCH 1/3] revlist.c: introduce --cherry for unsymmetric picking
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 18, 2011, 19:42 UTC
- Message-ID
- <7vzkptnn7x.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <15a90a6606cff7d823fe4afbedd580aadf7b1d1e.1298032360.git.git@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
Show 19 quoted lines
> 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@drmicha.warpmail.net>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.