-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Blocks/refactor twentytwentyone block processor (take 2) #10291
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
Open
dmsnell
wants to merge
11
commits into
WordPress:trunk
Choose a base branch
from
dmsnell:blocks/refactor-twentytwentyone-block-processor
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
e9f0d8a
Refactor twentwentyone to use Block_Processor
dmsnell 67b1cf7
Re-introduce legacy function for environments pre-6.9.0.
dmsnell 9015395
Adjust Block Processor logic to only scan top-level blocks.
dmsnell 9d100de
Recombine separate functions, harmonize logic.
dmsnell 35019d2
The freeform-check is redundant, already handled by the matching logic.
dmsnell 99968c4
Remove `str_starts_with()` and fix precedence defect.
dmsnell 52e0dd3
Revert legacy code changes, bring back str_starts_with
dmsnell 3216ca1
Minimize diff by undoing unnecessary change
dmsnell 26aebaa
Undo rename
dmsnell cfbff7f
wawawawa WPCS so much extra work
dmsnell 737b12f
By reusing match-text/prefix PHP can release $block_name earlier.
dmsnell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
The freeform-check is redundant, already handled by the matching logic.
- Loading branch information
commit 35019d2c8b3c1a6500f1905a82a071f21edcc0d6
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
str_starts_withtheoretically could be a problem in the legacy fallback if the site runs:(I do not recommend editing the legacy code because then you need to test it with legacy systems.)
Any site that supports
WP_Block_Processorshould supportstr_starts_withtoo.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.
in 99968c4 I removed
str_starts_with()because it’s easy enough to avoid it. theoretically it would be possible that someone manually adds theWP_Block_Processorclass on an old install without it.in the process I discovered a bug I had overlooked with precedence in the conditional. this has been resolved by splitting the conditional into two.
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.
and in 52e0dd3 I added it back, but added the feature gate. after undoing the legacy changes it didn’t seem as important to keep things coherent between them. I also misunderstood your comment initially because I overlooked that I introduced
str_starts_with()in the legacy updates.