revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-19 12:00:55 +01:00
Code Issues Releases Wiki Activity

Move object monogram rules later in the parse order

Browse source 
Summary:
Fixes T5837. The problem is that the hash is being recognized as a commit hash. We currently fire the object monogram rules fairly early, but there's no real reason to do this. Move them after all of the hyperlink rules:

    0 PhutilRemarkupEscapeRemarkupRule
    100 PhutilRemarkupMonospaceRule
    150 PhutilRemarkupDocumentLinkRule
    175 PhrictionRemarkupRule

  <<< OLD OBJECT RULE POSITION

    200 PhabricatorIconRemarkupRule
    200 PhabricatorMemeRemarkupRule
    200 DivinerSymbolRemarkupRule
    350 DoorkeeperRemarkupRuleJIRA
    350 PhabricatorYoutubeRemarkupRule
    350 DoorkeeperRemarkupRuleAsana
    400 PhutilRemarkupHyperlinkRule

  >>> NEW OBJECT RULE POSITION

    500 PhabricatorImageMacroRemarkupRule
    500 CustomInlineJIRA5Rule
    500 PhabricatorMentionRemarkupRule
    500 CustomInlineCodeRule
    1000 PhutilRemarkupDelRule
    1000 PhutilRemarkupBoldRule
    1000 PhutilRemarkupItalicRule
    1000 PhutilRemarkupUnderlineRule

  - The disadvantage of this approach is that `{F123, alt=go look at http://lol.com/ omg}` will parse the URL first, and then fail to resolve the object embed. This seems very rare / unusual.
  - The advantage is that all URLs which happen to have monograms in them work.

In the future, we could refine this by separating the rules, so the embed (`{...}`) versions fired at priority 200, while the normal versions fired at priority 450. We can wait for use cases, though. This is a little messy because the same code implements both rules.

Test Plan:
  - Verified example in T5837.
  - Marked up object rules like `F123` (works), `[[ asdf | F123 ]]` (works), `{F123, alt=http://example.com}` (does not work).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5837

Differential Revision: https://secure.phabricator.com/D10212
This commit is contained in:
epriestley 2014-08-11 12:29:23 -07:00
parent 448020a573
commit 98cd2cd799
1 changed files with 2 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php
View file

@ -9,7 +9,7 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule {
abstract protected function loadObjects(array $ids);
public function getPriority() {
return 200.0;
return 450.0;
}
protected function getObjectNamePrefixBeginsWithWordCharacter() {
@ -116,13 +116,11 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule {
}
// NOTE: The "(?<!#)" prevents us from linking "#abcdef" or similar.
// The "(?<!/)" prevents us from linking things in URIs.
// The "(?<!;)" prevents linking Diffusion URIs to commits.
// The "\b" allows us to link "(abcdef)" or similar without linking things
// in the middle of words.
$text = preg_replace_callback(
'((?<!#)(?<!/)(?<!;)'.$boundary.$prefix.'('.$id.')(?:#([-\w\d]+))?\b)',
'((?<!#)'.$boundary.$prefix.'('.$id.')(?:#([-\w\d]+))?\b)',
array($this, 'markupObjectReference'),
$text);