HomePhabricator

Reorder remarkup block rules for consistency with PhabricatorMarkupEngine

Description

Reorder remarkup block rules for consistency with PhabricatorMarkupEngine

Summary: I got consistent failures on the remarkup tests. This resolves that issue by bringing the order in line with the order used in PhabricatorMarkupEngine::newMarkupEngine

Test Plan: No more failures when running arc unit src/markup/engine/PhutilRemarkupEngine.php

Reviewers: epriestley, Blessed Reviewers

Reviewed By: epriestley, Blessed Reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15859

Event Timeline

Sorry, I wasn't exhaustive in my tests. It turns out this fixed behaviour for PHP 7, but broke it on 5. I'm investigating the difference.

The failed build is due to sort stability having been changed between 5 and 7. I'll send a diff to revert this, since phabricator doesn't support 7 officially. (T9640)

It's possible the tests are incorrect, although it would be good to understand why PHP5 and PHP7 differ.

One possible clue is T8929, which appeared to have some non-obivious behavioral difference in preg_match(). Remarkup uses a lot of regexps -- this is grasping at straws a bit, but is the only thing that springs to mind as spooky behavioral differences we haven't accounted for.

Do you know where the offending sort is?

Oh, $rules = msort($rules, 'getPriority');.

Yeah, let's revert this and I'll do the legwork to fix it properly since this is entirely my fault. We have a proper backward-compatible stable sort (msortv()) now but rules shouldn't have ambiguous priorities anyway.