yuja added a comment.
In https://phab.mercurial-scm.org/D451#7281, @quark wrote: > I think it's more correct if all core revsets support `defineorder` explicitly. The current code depends on `revset.makematcher` using ascending fullreposet as default to make `defineorder` implicitly functional. If the callsite passes an non-ascending (unordered, or descending) set to the returned matcher, the code would behave wrong (ex. `_flipand(1:0, _flipand(1:0, 0::1))` would be wrong if a descending fullreposet is passed). Perhaps `_flipand(1:0, _flipand(1:0, 0::1))` would return `[1, 0]` if the input set were reversed. IMHO, that's correct under the original design. > It seems to me that the reason why people can get ordering wrong is because of legacy APIs (`repo, subset, x`). I tried to address that by introducing `intersect(subset, xs, order)` and suggest `repo, x` API (https://phab.mercurial-scm.org/D453). If people write code using the new API, it's much harder to have ordering issues. > > https://phab.mercurial-scm.org/D456 provides a good test coverage about core revset ordering issues. If we really want to address ordering issues of 3rd party code, maybe we can deprecate `repo, subset, x` API and force people to either take `subset` and `order`, or none of them. Or require an explicit `supportorder` flag to be defined to mark it as order-safe. I agree new decorator API would be slightly better for trivial cases, but the situation for 3rd-party extensions would be worse. Before, `subset` was the canonical source of ordering. Since most revset predicates should not have their own order, they just needed to return a set in subset's order. return subset.filter(...) IIUC, this will be no longer be valid. All revset predicates will have to take care of `order` flag if they need a `subset` argument, just because few predicates want to enforce their order. This seems not a good balance. INLINE COMMENTS > revset.py:55 > > -def getset(repo, subset, x): > +def getset(repo, subset, x, order=anyorder): > if not x: Perhaps the default `order=defineorder` would be safer at this point. > quark wrote in revset.py:893 > In this case, `y` is expected to completely redefine the order. So `y`'s > `subset`'s order does not matter. So in your proposed design, that's true. x's order doesn't matter. I just meant, in the original design, `x` should follow the subset's order because `y` could have no explicit ordering (so `y` follows `x`, which follows `subset`.) REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D451 To: quark, #hg-reviewers Cc: yuja, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel