Editor: prepend_to_selector: optimized with str_replace()#12306
Editor: prepend_to_selector: optimized with str_replace()#12306josephscott wants to merge 1 commit into
Conversation
|
Hi there! 👋 Thank you for your contribution to WordPress! 💖 It looks like this is your first pull request to No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making. More information about how GitHub pull requests can be used to contribute to WordPress can be found in the Core Handbook. Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook. If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook. The Developer Hub also documents the various coding standards that are followed:
Thank you, |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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. |
|
@josephscott I’ve added the full set of start characters here to remove the bug this introduces, and I’m going to add the expanded |
|
@josephscott mind giving this a look before I merge it? the changes that I made are:
|
Trac ticket: https://core.trac.wordpress.org/ticket/65533 This PR syncs the foloowing Gutenberg PRs: - WordPress/gutenberg#76556 - WordPress/gutenberg#79499 Opus 4.6 used in the initial patch creation in Gutenberg. Co-Authored-By: Dennis Snell <dmsnell@git.wordpress.org>
dmsnell
force-pushed
the
editor/optimize-prepend-to-selector
branch
from
a41df2d to
7a34811
Compare
|
Rebased in preparation for merge: previous HEAD was a41df2d |
|
@dmsnell my apologies for not getting to this sooner. I have no problem with these changes in the effort to avoid breaking things. What I'm not sure about is using |
|
thanks @josephscott. I did anticipate that we’d lose some benefit in using the reason for the change was consistency with the interface and a desire to reduce the risk of downstream code making assumptions about what this function produces. initially I left in the optimization you had, but then the behavior became inconsistent with respect to whitespace and the test suites demonstrated that. happy to update this if we find regressions, at which point we should re-examine again the goal of |
Splitting a CSS selector involves unnecessary computation and allocation when the selector could not possibly involve more than one selector. It was discovered in profiling that this function was being called 4000+ times in a single request, and adding a pre-condition before splitting saved meaningful time on a page render. This patch adds the pre-condition to verify that any commas present in a selector string can only be “comma tokens,” which split selectors at the top level, making it safe to fall back to simpler parsing that’s more efficient than general separator splitting. This commit synchronizes work from the following Gutenberg PRs: - WordPress/gutenberg#76556 - WordPress/gutenberg#79499 Developed in: #12306 Discussed in: https://core.trac.wordpress.org/ticket/65533 Follow-up to [62607]. Props dmsnell, josephscott. See #65533. git-svn-id: https://develop.svn.wordpress.org/trunk@62650 602fd350-edb4-49c9-b593-d223f7449a82
Splitting a CSS selector involves unnecessary computation and allocation when the selector could not possibly involve more than one selector. It was discovered in profiling that this function was being called 4000+ times in a single request, and adding a pre-condition before splitting saved meaningful time on a page render. This patch adds the pre-condition to verify that any commas present in a selector string can only be “comma tokens,” which split selectors at the top level, making it safe to fall back to simpler parsing that’s more efficient than general separator splitting. This commit synchronizes work from the following Gutenberg PRs: - WordPress/gutenberg#76556 - WordPress/gutenberg#79499 Developed in: WordPress/wordpress-develop#12306 Discussed in: https://core.trac.wordpress.org/ticket/65533 Follow-up to [62607]. Props dmsnell, josephscott. See #65533. Built from https://develop.svn.wordpress.org/trunk@62650 git-svn-id: http://core.svn.wordpress.org/trunk@61935 1a063a9b-81f0-0310-95a4-ce76da25c4cd
2 participants
Trac ticket: https://core.trac.wordpress.org/ticket/65533
This PR syncs the Gutenberg PRs:
Use of AI Tools
AI assistance: Yes
Tools: Claude
Model: Opus 4.6
Used for: digging into php-spx results and generating optimization ideas
prepend_to_selector()tests toappend_to_selector()tests.This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.