D451: revset: remove order information from tree

yuja (Yuya Nishihara) phabricator at mercurial-scm.org
Wed Aug 23 09:11:52 EDT 2017


yuja added a comment.


  In https://phab.mercurial-scm.org/D451#7499, @quark wrote:
  
  > hgsubversion:
  >
  > - svnrev() uses `x for x in myset if x in subset` therefore enforces "defineorder" and wrong.
  > - fromsvn() is also wrong similarly.
  
  
  Yes, they are wrong (and I've pointed out that before in hgsubversion thread.)
  They could benefit from new `subset`-less API.
  
  > hg-git:
  > 
  > - fromgit(), gitnode(): happened to be "followorder" since it uses `x for x in subset if ...`
  > 
  >   mutable-history:
  > - troubled(), suspended(), precursors(), ...: all of them use `subset & ...` , therefore enforces "followorder"
  > 
  >   remotenames:
  > - remotenames(): uses `subset & ...` pattern
  > - upstream(): uses `smartset.filteredset(subset, ...)`, which could be changed to `subset.filter(...)`. But it's literally `subset & tipancestors` and does not have to use `subset.filter`.
  > - pushed(): same as upstream()
  > 
  >   All of them could just remove `subset` argument without hurting performance.  They may not be able to do that because they need to support older Mercurial. But I think that could be seen as an indication that new code //probably// does not need the `subset` argument.
  
  The point is these implementations would become invalid if `subset` were
  optimized to "any" order. That's breaking change, but which is likely to be
  not covered by tests.
  
  As I said, most revset predicates have no explicit order. I introduced the
  order flag in order to work around a few exceptions, which are `x:y`,
  `x + y`, `sort()` and `reverse()`, IIRC. A strong "define" is exceptional.
  
  So, is it really make sense to revamp the revset ordering rules? I don't
  think so. I generally like this series, but -1 for bringing "any" order
  everywhere.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D451

To: quark, #hg-reviewers
Cc: martinvonz, yuja, mercurial-devel


More information about the Mercurial-devel mailing list