Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> writes:
> Junio C Hamano venit, vidit, dixit 09.03.2011 22:49:
> ...
>> Conceptually revs->cherry_mark ought to be a subset of revs->cherry_pick
>> and the code shouldn't have to do something like this:
>>
>> if (revs->cherry_pick || revs->cherry_mark)
>> cherry_pick_list();
>>
>> Instead, the code should arrange that revs->cherry_pick is always set
>> when revs->cherry_mark is set before the calling application enters the
>> loop to call get_revision().
>>
>> But that would make the command line parsing more cumbersome (you would
>> either waste one bit so that you can tell if you saw --cherry-pick on the
>> command line, or keep the version of parser in this series as-is, and add
>> postprocessing code to flip revs->cherry_pick on when revs->cherry_mark
>> was given in prepare_revision_walk()), and I understand that is why you
>> did it that way?
>
> Yes, I think so :) Another reason is that even cherry_pick_list() needs
> to know which object flag it should set.
I didn't have trouble with "if (revs->cherry_mark)" used for that purpose
at all.
That is what I meant by "cherry-mark is a _subset_ of cherry-pick". In
other words, if you are marking the cherry-pick result, you must be doing
a cherry-pick to begin with, so having to say "cherry-pick || cherry-mark"
is a bad taste.
But that does not contradict with the need to do something differently
between the case where you are cherry-marking and where you are not
cherry-marking.
> - leave the parser as is
> - set cherry_pick when cherry_mark is set
> - if (revs->cherry_pick) cherry_pick_list();
> - leave cherry_pick_list() as is
I think that essentially is what I speculated in "either X or Y" part as
solution Y.
> Would that be better? I didn't do that because it changes the meaning of
> cherry_pick (it can mean two things then). It would introduce the
> assumption "pick is set when mark is" instead of "at most one is set"
Well, as I said earlier, my suggestion was coming from "if you are
cherry-marking, you must be cherry-picking in the first place", so yes, I
am saying that "at most one is set" is a bad taste. If the assumption has
to change to match the better taste, that can only be a good thing ;-).
> Any thoughts on the actual marks ('+' vs. ' ' vs. '*')?
> I also feel like we could use a space between the mark and the sha1
> (like git cherry does) to ease copy and paste but that is an orthogonal
> issue applying to all marks (<>-^+=).
I personally don't care very much and also choose not to say much on it
too early even if I had opinion on the choice, because I know there are
many people with better visual tastes than I on the list and I am sure
they will voice their opinion once they see sample output.
--
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