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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 21, 2011, 19:49 UTC
Message-ID: <7vmxlpi2vt.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4D625972.4090500@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

Show 7 quoted lines

>> 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 don't understand this remark - isn't that exactly what you suggest
> below ("--right-only")? Then what is distasteful and short-sighted?

It does not give people an option to ask for only left side if you give only "--cherry" and nothing else. Don't you find that a bad taste?

Compare that with being explicit and supporting both --right/left-only. The point is to avoid making the internal implementation and plumbing interface needlessly limited to one use case, and instead keeping them flexible.

> Could you explain in what way the current implementation is not done
> "properly"?

Your "cherry" implementation limits its utility unnecessarily by assuming a few things it does not have to assume:

 - It only needs to discard left hand side, but not right hand side.
 - It only needs to work when patch-id equivalence filtering is on.

The former is easy to rectify, I think.

Right now, there is only one thing (cherry-pick) that works on a symmetric difference set and filter, so it may _appear_ that the latter does not matter, but by not tying the two unrelated concepts together and keeping the "right-side-only output" logic and "filter the symmetric difference set" logic orthogonal, you will leave the door open for others to add different filtering mode easier.

> So I think the plan is:
>
> - use a "right-only" flag rather than "cherry"

And perhaps left-only for symmetry.

> - make "--cherry" do "--cherry-pick --right-only" (with or without
> ".."->"...")

Yes.

> - simplify cmd_format_patch

As long as the internal machinery to run the right/left-only filtering of the result of cherry-pick filtering is done (i.e. step 1) and then the syntactic sugar --cherry synonym is implemented in terms of it (step 2), cmd_format_patch simplification would come as a natural consequence, I think.

The simplification does not have to be done as part of this series, though. I only mentioned it as a reason why we would want to get the internal for this topic (step 1) right, not tied too strongly to step 2.

Thanks.