[PATCH 3 of 7 V4] revset: insert _unordered to fix ordering of nested 'or' and '_list*()' (BC)

Yuya Nishihara yuya at tcha.org
Sun Jun 26 04:33:55 EDT 2016


On Sat, 25 Jun 2016 06:26:17 -0700, Martin von Zweigbergk wrote:
> On Fri, Jun 24, 2016 at 10:58 PM, Yuya Nishihara <yuya at tcha.org> wrote:
> > On Fri, 24 Jun 2016 10:27:03 -0700, Martin von Zweigbergk wrote:  
> >> On Thu, Jun 23, 2016 at 8:59 AM, Yuya Nishihara <yuya at tcha.org> wrote:  
> >> > # HG changeset patch
> >> > # User Yuya Nishihara <yuya at tcha.org>
> >> > # Date 1455629461 -32400
> >> > #      Tue Feb 16 22:31:01 2016 +0900
> >> > # Node ID bd5e886ceca4257dd843f98539a18ef7c31ec8e0
> >> > # Parent  0a1b1af8c9b8aaa0690de2fc0614b531dc03a5c2
> >> > revset: insert _unordered to fix ordering of nested 'or' and '_list*()' (BC)  
> >  
> >> Getting the order right is not an optimization, so I'd really like the
> >> order to be fixed before optimize(), and then optimize() can (as you
> >> do in later patches) remove some unnecessary ordering.  
> >
> > optimize() does a bunch of tree rewriting stuffs unrelated to the
> > optimization. Do you mean we should split optimize() into two more parts
> > so that we can compare the optimized result with the pure result?  
> 
> It seems to me like that would make it clearer if optimize() only did
> optimization, and I had assumed that that's how it already was. Since
> that is far from the case, that would obviously be another patch if we
> even want to do that.

Sure. I like the idea of splitting optimize(). If it is simple enough, let's
try that as a separate series.

> > I'm okay to add unorderedor(), _unorderdlist*(), etc. and replace _unordered()
> > by them. That will avoid confusion.  
> 
> Yes, I think that would make it much clearer that they were not
> following the subset ordering like the other functions do. Thanks!

will do in V5. BTW, it seems we have opposite definition for these terms.
My definition is:

  ordered:   has special ordering like rangeset() and orset()
  unordered: has no special requirement for ordering, follows the subset

From your comment, I should rename the current _list to _(un)?orderedlist
to mark it a function having special ordering requirement.


More information about the Mercurial-devel mailing list