Page MenuHomePhabricator

Specialize rendering of self-URIs in the form "/X123"

Authored by epriestley on May 9 2019, 8:34 PM.



Depends on D20510. Ref T5378. When remarkup includes a hyperlink to the current install in the form "/X123" (which is common), load the corresponding object and specialize the rendering.

This doesn't cover everything (notably, no handling for Diffusion paths yet), but does cover a lot of the most common cases.

The "uri" form preserves the URI as written, but adds an icon, tag, and hovercard.

The "{uri}" form is more similar to {T123} and shows the object name.

Test Plan

Diff Detail

rP Phabricator
Lint OK
Unit Tests OK
Build Status
Buildable 22778
Build 31243: Run Core Tests
Build 31242: arc lint + arc unit

Unit TestsFailed

199 msPhabricatorLibraryTestCase::testEverythingImplemented
EXCEPTION (PhutilMissingSymbolException): Failed to load class/interface "PhabricatorSelfHyperlinkEngineExtension". The symbol map for library "phabricator" (at "/core/data/drydock/workingcopy-90/repo/phabricator/src") claims this symbol (of type "class/interface") is defined in "applications/meta/engineextension/PhabricatorSelfHyperlinkEngineExtension.php", but loading that source file did not cause the symbol to become defined.
1,046 msPhabricatorLibraryTestCase::testMethodVisibility
EXCEPTION (PhutilMissingSymbolException): Failed to load class or interface "PhutilRemarkupHyperlinkEngineExtension". The class or interface "PhutilRemarkupHyperlinkEngineExtension" is not defined in the library map of any loaded library.
1 msAlmanacNamesTestCase::testServiceOrDeviceNames
30 assertions passed.
0 msAlmanacServiceTypeTestCase::testGetAllServiceTypes
1 assertion passed.
0 msAphrontHTTPSinkTestCase::testHTTPHeaderNames
2 assertions passed.
View Full Test Results (2 Failed · 362 Passed)

Event Timeline

epriestley created this revision.May 9 2019, 8:34 PM

Slightly more complete test case:

Harbormaster returned this revision to the author for changes because remote builds failed.May 9 2019, 8:35 PM
Harbormaster failed remote builds in B22778: Diff 48905!
epriestley requested review of this revision.May 9 2019, 8:40 PM

(Test failures are because of D20511 not being landed yet.)

joshuaspence accepted this revision.May 14 2019, 12:36 AM
joshuaspence added a subscriber: joshuaspence.
joshuaspence added inline comments.

A minor point, but this will match /robots.txt... should the regex be tightened so as to only allow monograms?


I guess it doesn't really matter since you are looking up the objects anyway.

This revision is now accepted and ready to land.May 14 2019, 12:36 AM
epriestley added inline comments.May 16 2019, 6:25 PM

Yeah, we'll match various patterns, and /D99999999999999, but I expect these to be pretty rare and the behavior will still be correct, we'll just do a little extra work (discarding thing as a possible object name, or querying for D99999999999 and failing to load it).

This revision was landed with ongoing or failed builds.May 16 2019, 7:16 PM
This revision was automatically updated to reflect the committed changes.