Ref T9722, Add Phurl Remarkup as ((id)) or ((alias))
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T9722: Add Phurl Remarkup
- Commits
- Restricted Diffusion Commit
rP268fac25d544: Add Phurl Remarkup
Add a comment to any object as ((id)) or ((alias)). Make sure comment renders as a link.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
This looks good, but see a few inlines for additional behaviors.
| src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php | ||
|---|---|---|
| 14 | I think the character class in the middle should include space at a minium ([^ )]) so that ((blah blah blah)) (with actual spaces) doesn't trigger. Possibly we should get a little stricter about it so that ((^_^)) doesn't trigger, although I don't know how many false positives we're really going to see. But it would be bad if stuff like if ((a == b) || (c == d)) {... triggered it -- that one won't since it has a ) inside it, but This isn't really a big deal since ((...)) is uncommon even in programming, but I think we should be slightly stricter than this. | |
| 19 | We should include these checks: An if (!$this->isFlatText($matches[0])) test, like IconRemarkupRule has. This prevents us from doing bad stuff with input like this: (([[a|b]])) That should render as ((b)), with b as a link to a, not as a Phurl. A text mode check. Maybe return text like this? See "URL Name" <https://install.com/u/example> An HTML mail mode check, which can probably just render a normal link. | |
| 38 | I'd tend to expect this to be a TagView (like Txxx), not an IconView (like {icon ...}). | |
| 44 | This should just return the original text -- if I type ((zebra)) or ((289379827)) and there's no such link (or the viewer can't see it) we should just return the text as-is. | |
| src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php | ||
|---|---|---|
| 38 | Is there a way to make the TagView look like an actual link? | |
Comment Actions
You can just use phutil_tag('a', ...) if you want a plain link. I don't have a strong opinion on a plain link vs an object reference style tag thing.
| src/applications/phurl/remarkup/PhabricatorPhurlLinkRemarkupRule.php | ||
|---|---|---|
| 23 | $mail_mode is never actually used to render a mail version, although that's fine now that we're just rendering a normal link. But you could get rid of the call here since the mail and HTML modes are the same now. | |