-
Notifications
You must be signed in to change notification settings - Fork 3.2k
HTML API: Use Tag Processor when adding rel keywords to A elements.
#9252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
1b29d36 to
a6d758a
Compare
Prep work for WordPress#9252. Part of WordPress#9248.
) Prep work for WordPress#9252. Part of WordPress#9248.
) Prep work for WordPress#9252. Part of WordPress#9248.
| * @since {WP_VERSION} | ||
| * | ||
| * @param string $attribute_value HTML-decoded attribute value to parse. | ||
| * @param string $case_sensitivity Optional. Constrain uniqueness with 'case-sensitive' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious why this is a string param and not a boolean?
If going with a string, then this could use 'case-sensitive'|'case-insensitive' as the type instead of string, in alignment with the proposal to adopt PHPStan. This would add static type checking for bad string values. If string values are used as well, maybe they should be added as constants to the class so that the literals aren't passed around everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they are there for discoverability in the code, plus I believe that in a very inconsequential amount the checking of string equality is faster than even booleans, because PHP short-circuits casting
100% it’s there because I find boolean parameters opaque and string parameters are explicit.
happy to update to use the string values in the types, but I thought WPCS nags yapped at me in the past because I tried that and it wanted string instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, performance is not my concern here. I'm more concerned with typos, and the DX of having to type in an exact string and what happens if you get it wrong.
Alternatively, this could take an $options array which has a case_sensitive key with a boolean value. This would seem more WordPressy and would allow for more flexibility in the future to add more options without adding additional positional params.
a6d758a to
bc2ca29
Compare
) Prep work for WordPress#9252. Part of WordPress#9248.
) Prep work for WordPress#9252. Part of WordPress#9248.
Prep work for WordPress#9248 See also WordPress#9251
bc2ca29 to
a8dd424
Compare
) Prep work for WordPress#9252. Part of WordPress#9248.
) Prep work for WordPress#9252. Part of WordPress#9248.
) Prep work for WordPress#9252. Part of WordPress#9248.
) Prep work for WordPress#9252. Part of WordPress#9248.
Prep work for #9248
See also #9251, #9255