From: Junio C Hamano Date: Wed, 15 Aug 2012 18:39:05 GMT Subject: Re: [PATCH 2/4] revisions passed to cherry-pick should be in "default" order Message-ID: <7vr4r857au.fsf@alter.siamese.dyndns.org> In-Reply-To: Martin von Zweigbergk writes: > Makes sense, I'll try to implement it that way. I was afraid that > we would need to call prepare_revision_walk() once first and then > if we afterwards find out that we should not walk, we would need > to call it again without the reverse option. > But after looking at > how rev_info.reverse is used, it seem like it's only used in > get_revision(), so we can leave it either on or off during the > prepare_revision_walk() and the and set appropriately before > calling get_revision(), like so: > > init_revisions(&revs); > revs.no_walk = REVISION_WALK_NO_WALK_UNSORTED; > setup_revisions(...); > prepare_revision_walk(&revs); > revs.reverse = !revs.no_walk; Sorry, but I do not understand why you frutz with "reverse" after prepare, and not before. I think you can just set no_walk and let setup_revisions() turn it off upon seeing a range (this happens in add_pending_object()). After setup_revisions() returns, if no_walk is still set, you only got individual refs without ranges, so no reversing required. You also need to be careful about "revert" that shares the code; when reverting range A..C in your example, you want to undo C and then B, and you do not want to reverse them.