From 98cd2cd7992a468906b2059388ef65368af91e86 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 11 Aug 2014 12:29:23 -0700
Subject: [PATCH] Move object monogram rules later in the parse order

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
---
 .../markup/rule/PhabricatorObjectRemarkupRule.php           | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php
index e4021830a0..3cc7e13c48 100644
--- a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php
+++ b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php
@@ -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);