Skip to content

Conversation

@dmsnell
Copy link
Member

@dmsnell dmsnell commented Jul 12, 2025

Prep work for #9248
See also #9251, #9255

@github-actions
Copy link

Test using WordPress Playground

The 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

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@dmsnell dmsnell force-pushed the html-api/refactor-wp-rel-nofollow branch from 1b29d36 to a6d758a Compare July 13, 2025 18:14
dmsnell added a commit to dmsnell/wordpress-develop that referenced this pull request Jul 13, 2025
dmsnell added a commit to dmsnell/wordpress-develop that referenced this pull request Jul 13, 2025
dmsnell added a commit to dmsnell/wordpress-develop that referenced this pull request Jul 13, 2025
* @since {WP_VERSION}
*
* @param string $attribute_value HTML-decoded attribute value to parse.
* @param string $case_sensitivity Optional. Constrain uniqueness with 'case-sensitive'
Copy link
Member

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.

Copy link
Member Author

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

Copy link
Member

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.

@dmsnell dmsnell force-pushed the html-api/refactor-wp-rel-nofollow branch from a6d758a to bc2ca29 Compare July 15, 2025 02:11
dmsnell added a commit to dmsnell/wordpress-develop that referenced this pull request Jul 15, 2025
dmsnell added a commit to dmsnell/wordpress-develop that referenced this pull request Jul 15, 2025
@dmsnell dmsnell force-pushed the html-api/refactor-wp-rel-nofollow branch from bc2ca29 to a8dd424 Compare July 15, 2025 13:31
dmsnell added a commit to dmsnell/wordpress-develop that referenced this pull request Jul 15, 2025
dmsnell added a commit to dmsnell/wordpress-develop that referenced this pull request Jul 15, 2025
dmsnell added a commit to dmsnell/wordpress-develop that referenced this pull request Jul 15, 2025
dmsnell added a commit to dmsnell/wordpress-develop that referenced this pull request Jul 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants