[PATCH 3 of 3 V2] revsets: add new toposort predicate

[Yuya Nishihara]

On Fri, 20 May 2016 01:56:41 +0200, Pierre-Yves David wrote:
> On 05/19/2016 08:11 PM, Martijn Pieters wrote:
> > On 19 May 2016 at 01:29, Pierre-Yves David
> > <pierre-yves.david at ens-lyon.org> wrote:  
> >>> Rather than cram toposort into the sort() but then have to explain
> >>> that when you use 'topo' as a sort key the other keys must be ignored,
> >>> it was much cleaner to create a separate predicate.  
> >> I'm not really convinced by that argument. Actually I think it does make
> >> sense. The topological sort will eventually grow some way to break tie.
> >> Being able to says things like "topo -date" would allow to express them
> >> (here, sort more recent first in case of tie). So using the "sort()"
> >> predicate is actually helping to solve one of our UI challenge with
> >> topological sorting.
> >> However, we should probably disallow "topo" anywhere but in first
> >> position (at least at first).
> >>  
> >>> toposort() also needs to accept the firstbranch argument; it would
> >>> have required adding a third to sort() that only applies to just one
> >>> type of sort.  
> >> I find this argument a bit more convincing. Having a parameter for a
> >> single case would be inelegant (not impossible but not super excited
> >> about it).
> >>
> >> Yet, maybe we can express this parameters in a way that is useful for
> >> other kind of sorting. If we turn this into some kind of manual priority
> >> it would make senses for others:
> >>
> >> - topo → branch(es) to display first,
> >> - author → manual list of priority/order for author name. Useful to put
> >> oneself first,
> >> - branch → idem, having the ability to manually sort branch (eg: stable  
> >>>> default).  
> >> This would be especially useful in combination with the ability to
> >> provide multiple sort key (eg "topo author") as you could very precisely
> >> specify how to break tie.  
> > Interesting, that does make a compelling argument to make 'topo' a
> > special kind of `sort()` key.
> >
> > But the next question then would be: how should per-sort-key arguments
> > then be expressed?
> >
> > I wonder if predicates could support *keyword arguments* here:
> >
> >     sort(set, "author -date', author=mjpieters at fb.com)
> >
> > would sort by author first (putting me at the top), then by reverse
> > date for example. Each sort key accepts just one keyword argument, and
> > sort keys must be unique (sorting by the same key twice makes no sense
> > anyway).
> >  
> >> That said, there is probably other parameters we might want to implement
> >> in the future. For exaple some limit to the size of a topological
> >> grouped series of revs (to avoid sorting too far down in case of -very-
> >> long mixed branches). Having such parameter may not make sense for most
> >> of the key (or would deserve different default). This is more a generic
> >> statement about how we could have more option than a specific example to
> >> study.  
> > Going along with the keyword arguments, you could extend this to use prefixes:
> >
> >     topo_firstbranch=...
> >     topo_grouplimit=...
> >
> >     author_first=...
> >
> >     branch_first=...

or we might want to extend the key arguments to support function-like syntax:

% hg debugrevspec -v 'sort(a, topo, branch(first=stable))'
(func
  ('symbol', 'sort')
  (list
    ('symbol', 'a')
    ('symbol', 'topo')
    (func
      ('symbol', 'branch')
      (keyvalue
        ('symbol', 'first')
        ('symbol', 'stable')))))

It will require some hack in optimize().

> > That said, expanding the parser to support keyword arguments is a
> > little out of my reach and time budget. *If* I were to merge
> > toposort() into sort(), would using the 3rd argument work until such
> > time that alternative syntax is found (after which a 3rd argument is,
> > for a while, an alias for the new syntax spelling)?  
> 
> Actually, I though, they already supported it

Yeah, it does. Example:

https://selenic.com/repo/hg/file/tip/mercurial/revset.py#l905

BTW, I have unsent patches to factor out big "if"s from sort(). If it will
conflict with your "topo" sort by design, I'll drop my patches.

https://bitbucket.org/yuja/hg-work/commits/30d426bd6dbe