Page MenuHomePhabricator

Move object monogram rules later in the parse order
ClosedPublic

Authored by epriestley on Aug 11 2014, 1:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Aug 27, 3:57 AM
Unknown Object (File)
Sun, Aug 25, 5:05 PM
Unknown Object (File)
Fri, Aug 23, 12:39 AM
Unknown Object (File)
Fri, Aug 23, 12:39 AM
Unknown Object (File)
Thu, Aug 22, 11:55 AM
Unknown Object (File)
Sun, Aug 18, 4:52 AM
Unknown Object (File)
Sat, Aug 17, 11:06 PM
Unknown Object (File)
Aug 8 2024, 5:36 AM
Subscribers

Details

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).

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley retitled this revision from to Move object monogram rules later in the parse order.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
This revision is now accepted and ready to land.Aug 11 2014, 6:30 PM
epriestley updated this revision to Diff 24597.

Closed by commit rP98cd2cd7992a (authored by @epriestley).