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