On Fri, Jan 13, 2012 at 12:55:58AM +0530, Ramkumar Ramachandra wrote: > Junio C Hamano wrote: > > You should be able to look at revs->cmdline and tell if you need to let > > cherry-pick walk (i.e. "cherry-pick master..next"), or if the user wants > > individual commits (i.e. "cherry-pick A B C"). > > > > And you do prepare_revision_walk() only when you need to walk; otherwise > > you use the contents of revs->pending in order. I am tempted to suggest that cherry-pick should not feed its arguments to the revision machinery in the first place, but instead accept a set of arguments, each argument of which is either a single commit (interpreted by get_sha1) or a range specifier (which can be fed to setup-revisions). And then get a linearized set of commits for _each_ argument independently and concatenate them (possibly eliminating duplicates). That would make all of these work as most people would expect: git cherry-pick A B C git cherry-pick A..B git cherry-pick A..B B..C but would be a regression for: git cherry-pick B ^A versus the current code. I suspect that the latter form is not all that commonly used, though, and certainly I would accept it as a casualty of making the "A B C" form work. My only hesitation is that it is in fact a regression. > Okay, just to make sure I understand this correctly: if more than one > argument is literally specified, I should not set up the revision > walker and pick the commits listed in revs->pending, correct? Then, > when I encounter the following command, > > $ git cherry-pick maint ^master > > I should just pick two commits: maint, and ^master. But ^master is not a commit, it is a negation. So it is nonsensical if the arguments are considered independent of each other (you _could_ use a heuristic to guess that they are not independent, but I'd rather not go there). So you'd probably end up just rejecting the arguments. -Peff -- 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