Patchwork [2,of,2] log: drop hack to fix order of revset (issue5100)

login
register
mail settings
Submitter Yuya Nishihara
Date Sept. 17, 2016, 10:17 a.m.
Message ID <f99625523f84eaaedf48.1474107460@mimosa>
Download mbox | patch
Permalink /patch/16661/
State Accepted
Delegated to: Ryan McElroy
Headers show

Comments

Yuya Nishihara - Sept. 17, 2016, 10:17 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1462253040 -32400
#      Tue May 03 14:24:00 2016 +0900
# Node ID f99625523f84eaaedf48897322d5c9f6dce64bd9
# Parent  961b53cbadd0e17b1e5e745d0e3cc582cbf6603f
# EXP-Topic revsetflag
log: drop hack to fix order of revset (issue5100)

Specify ordered=True instead.

This patch effectively backs out c407583cf5f6. revs.sort(reverse=True)
is replaced by revs.reverse() because the matcher should no longer reorder
revisions.
Yuya Nishihara - Sept. 17, 2016, 10:24 a.m.
On Sat, 17 Sep 2016 19:17:40 +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1462253040 -32400
> #      Tue May 03 14:24:00 2016 +0900
> # Node ID f99625523f84eaaedf48897322d5c9f6dce64bd9
> # Parent  961b53cbadd0e17b1e5e745d0e3cc582cbf6603f
> # EXP-Topic revsetflag
> log: drop hack to fix order of revset (issue5100)
> 
> Specify ordered=True instead.

Oops, this should be s/ordered=True/order=revset.followorder/.
Pierre-Yves David - Sept. 20, 2016, 3:15 p.m.
On 09/17/2016 12:17 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1462253040 -32400
> #      Tue May 03 14:24:00 2016 +0900
> # Node ID f99625523f84eaaedf48897322d5c9f6dce64bd9
> # Parent  961b53cbadd0e17b1e5e745d0e3cc582cbf6603f
> # EXP-Topic revsetflag
> log: drop hack to fix order of revset (issue5100)
>
> Specify ordered=True instead.

Pushed (I've fixed the message in flight)

> This patch effectively backs out c407583cf5f6. revs.sort(reverse=True)
> is replaced by revs.reverse() because the matcher should no longer reorder
> revisions.

Given that most expensive matching is now lazy, (so actually happening 
after the actual match call, should we drop the double reverse all entirely?

Cheers,

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2189,19 +2189,10 @@  def getlogrevs(repo, pats, opts):
         # order, because most filters deal with the changelog.
         if not opts.get('rev'):
             revs.reverse()
-        matcher = revset.match(repo.ui, expr)
-        # Revset matches can reorder revisions. "A or B" typically returns
-        # returns the revision matching A then the revision matching B. Sort
-        # again to fix that.
-        fixopts = ['branch', 'only_branch', 'keyword', 'user']
-        oldrevs = revs
+        matcher = revset.match(repo.ui, expr, order=revset.followorder)
         revs = matcher(repo, revs)
         if not opts.get('rev'):
-            revs.sort(reverse=True)
-        elif len(pats) > 1 or any(len(opts.get(op, [])) > 1 for op in fixopts):
-            # XXX "A or B" is known to change the order; fix it by filtering
-            # matched set again (issue5100)
-            revs = oldrevs & revs
+            revs.reverse()
     if limit is not None:
         limitedrevs = []
         for idx, r in enumerate(revs):