Page MenuHomePhabricator

Remarkup links to link to short url instead of long and fix commenting on Phurl's
ClosedPublic

Authored by lpriestley on Nov 13 2015, 10:54 PM.
Tags
None
Referenced Files
F13304856: D14477.id35017.diff
Sat, Jun 8, 1:46 PM
F13281316: D14477.diff
Sun, Jun 2, 10:32 AM
F13268558: D14477.diff
Wed, May 29, 5:10 AM
F13265467: D14477.id35079.diff
Tue, May 28, 4:41 AM
F13256864: D14477.id35079.diff
Sat, May 25, 6:36 PM
F13256860: D14477.id35017.diff
Sat, May 25, 6:35 PM
F13255941: D14477.id35017.diff
Sat, May 25, 9:12 AM
F13255940: D14477.diff
Sat, May 25, 9:12 AM
Subscribers

Details

Summary

Ref T6049, remarkup links to use short URLs and make commenting on Phurl's actually work

Test Plan
  • Create Phurl U123
  • Comment on that Phurl ((123))

Comment should link to /u/123

Diff Detail

Repository
rP Phabricator
Branch
phurllinkredirect
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 8827
Build 10299: Run Core Tests
Build 10298: arc lint + arc unit

Event Timeline

lpriestley retitled this revision from to Remarkup links to link to short url instead of long and fix commenting on Phurl's.
lpriestley updated this object.
lpriestley edited the test plan for this revision. (Show Details)
lpriestley added a reviewer: epriestley.
epriestley edited edge metadata.
epriestley added inline comments.
src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php
10

(Possibly we should make ((U123)) and ((123)) equivalent at some point, or maybe even require the U... form.)

44–45

I think these should probably become methods on PhurlURL, like getDisplayName() and getRedirectURI() or similar: specifically, I think they'll have more callsites.

The $redirect logic should also probably be like this?

if (strlen($phurl->getAlias())) {
  return '/u/alias';
} else {
  return '/u/id';
}
This revision is now accepted and ready to land.Nov 15 2015, 4:44 PM
lpriestley marked an inline comment as done.
lpriestley edited edge metadata.

((U123)) should also remarkup as a redirect link to the URI for U123

This revision was automatically updated to reflect the committed changes.