Changeset View
Standalone View
src/infrastructure/markup/markuprule/PhutilRemarkupRule.php
Show First 20 Lines • Show All 100 Lines • ▼ Show 20 Lines | abstract class PhutilRemarkupRule extends Phobject { | ||||
* @param wild Ostensibly flat text. | * @param wild Ostensibly flat text. | ||||
* @return bool True if the text is flat. | * @return bool True if the text is flat. | ||||
*/ | */ | ||||
protected function isFlatText($text) { | protected function isFlatText($text) { | ||||
$text = (string)hsprintf('%s', phutil_safe_html($text)); | $text = (string)hsprintf('%s', phutil_safe_html($text)); | ||||
return (strpos($text, PhutilRemarkupBlockStorage::MAGIC_BYTE) === false); | return (strpos($text, PhutilRemarkupBlockStorage::MAGIC_BYTE) === false); | ||||
} | } | ||||
/** | |||||
* Get the CSS class="" attribute for a Remarkup link. | |||||
* It's just "remarkup-link" for all cases, plus the possibility for | |||||
* designers to style external links differently. | |||||
* @param boolean $is_internal Whenever the link was internal or not. | |||||
* @return string | |||||
*/ | |||||
protected function getRemarkupLinkClass($is_internal) { | |||||
// Allow developers to style esternal links differently | |||||
$classes = array('remarkup-link'); | |||||
if (!$is_internal) { | |||||
$classes[] = 'remarkup-link-ext'; | |||||
} | |||||
return implode(' ', $classes); | |||||
} | |||||
avivey: Lets move all of these new methods to a new class (and then make them non-static).
Also maybe… | |||||
Not Done Inline ActionsI understand. Two things: I think somewhere we already had this discussion and you suggested that not all URIs have an "is internal" concept. This may suggest to create a PhutilURIWeb class with these additional methods. In any case: touching PhutilURI or creating a PhutilURIWeb may be overkill changes since they are changes from another repo. Or, we can create this (or similar) new class in /phabricator to be less impactful, premising that a path and a name suggestion would be awesome. valerio.bozzolan: I understand. Two things:
I think somewhere we already had this discussion and you suggested… | |||||
} | } | ||||
Done Inline Actions✅ Exact equivalent of PhutilRemarkupDocumentLinkRule line 58 valerio.bozzolan: ✅ Exact equivalent of `PhutilRemarkupDocumentLinkRule` line 58 | |||||
Done Inline Actions✅ Exact equivalent of PhutilRemarkupDocumentLinkRule line 48 valerio.bozzolan: ✅ Exact equivalent of `PhutilRemarkupDocumentLinkRule` line 48 | |||||
Done Inline Actions✅ Just a commodity wrapper for general URLs in Remarkup valerio.bozzolan: ✅ Just a commodity wrapper for general URLs in Remarkup | |||||
Done Inline Actions✅ Just checks if something has the very same base URI or not. If you do not have base URI, it assumes as no as before. valerio.bozzolan: ✅ Just checks if something has the very same base URI or not. If you do not have base URI, it… | |||||
Done Inline Actions✅ Just a commodity method since str_starts_with() is available only after PHP 8 valerio.bozzolan: ✅ Just a commodity method since `str_starts_with()` is available only after PHP 8 | |||||
Done Inline Actionsyeah, ./ and ../ should also be considered internal (or possibly not allowed, for security reasons). avivey: yeah, `./` and `../` should also be considered internal (or possibly not allowed, for security… | |||||
Done Inline ActionsAdded. But the parser modification to deny these deserves a dedicated change. valerio.bozzolan: Added. But the parser modification to deny these deserves a dedicated change. | |||||
Not Done Inline Actions
speck: 1. I think it would be better to not include `./` and `../` here. Including these here reads as… | |||||
Done Inline ActionsEither use this method in all new cases where it applies (all new methods), or remove it. avivey: Either use this method in all new cases where it applies (all new methods), or remove it. | |||||
Not Done Inline ActionsSomething to make note of, if base-uri is https://we.phorge.it then someone making a link http://we.phorge.it/... would be considered external. I think that's fine (and considered "correct"), though we may want to document here that we're aware of it (and make note in the release notes). There isn't really a way to know that an install would have http routed to https even though that's likely the case in most setups (without attempting to resolve/contact the URI which we absolutely do not want to do). speck: Something to make note of, if `base-uri` is `https://we.phorge.it` then someone making a link… |
Lets move all of these new methods to a new class (and then make them non-static).
Also maybe make this a function of PhutilURI.