Block: Implement block insertion functions

[ockham]

See https://core.trac.wordpress.org/ticket/59313 and #5158.

Trac ticket: https://core.trac.wordpress.org/ticket/59385

---

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.

[ockham]

@gziolo I've decided to only implement insert_block_hook and inser_block_hooks here (plus unit test coverage). We'll only wire them to block templates and patterns to actually insert blocks in a follow-up 😄

Edit: I've updated the tracking ticket accordingly.

[ockham]

PHP 7.0 didn't like the trailing comma 😕

[gziolo]

Yes, that is a bit annoying. Usually, you need to put the string in its variable to use Heredoc:

Warning
Prior to PHP 7.3.0, it is very important to note that the line with the closing identifier must contain no other characters, except a semicolon (;). That means especially that the identifier may not be indented, and there may not be any spaces or tabs before or after the semicolon. It's also important to realize that the first character before the closing identifier must be a newline as defined by the local operating system. This is \n on UNIX systems, including macOS. The closing delimiter must also be followed by a newline.

So it's only the semicolon allowed which won't work here 😞

[ockham]

@gziolo There's another fix we might have to backport to GB: e9931e7.

[ockham]

@gziolo This needs test coverage for insert_hooked_blocks (plural) (see #5241 (comment)); otherwise, it might be about ready 🤞

[ockham]

Will need #5242 to eventually pass the parent arg to insert_hooked_blocks.

[gziolo]

insert_hooked_block seems ready to go.

[gziolo]

Yes, that is a bit annoying. Usually, you need to put the string in its variable to use Heredoc:

Warning
Prior to PHP 7.3.0, it is very important to note that the line with the closing identifier must contain no other characters, except a semicolon (;). That means especially that the identifier may not be indented, and there may not be any spaces or tabs before or after the semicolon. It's also important to realize that the first character before the closing identifier must be a newline as defined by the local operating system. This is \n on UNIX systems, including macOS. The closing delimiter must also be followed by a newline.

So it's only the semicolon allowed which won't work here 😞

[ockham]

I was considering doing one more thing: Split insert_hooked_block into insert_hooked_child_block and insert_hooked_sibling_block, since they're pretty different. I also might adjust the function signature of the latter a bit more to align with this.

[ockham]

I was considering doing one more thing: Split insert_hooked_block into insert_hooked_child_block and insert_hooked_sibling_block, since they're pretty different. I also might adjust the function signature of the latter a bit more to align with this.

Might actually rename to insert_child_block and insert_sibling_block, since they're arguably not even that specific to hooked blocks.

[ockham]

@gziolo Should be ready for review! I've changed it a lot, but I'm really happy with it now. It's much less specific to hooked blocks now, yet the functions are exactly the way we'll need them 😄

[gziolo]

I had a minor note about the return values that seem obsolete for the helper functions because they all mutate the first param, which is then returned. Anyway, I don't have strong feelings about it so if there is any reason to keep them, feel free to leave it as is.

[gziolo]

Test failures seem unrelated, so maybe there is a need to rebase the PR.

I like how the code is split now into smaller helper functions that are specialized and nicely encapsulate their task. One final note, they will be exposed in the dev docs portal, which I'm fine with.

[ockham]

Test failures seem unrelated, so maybe there is a need to rebase the PR.

Yeah, looks like unit tests have been failing for all recent PRs 😕

I like how the code is split now into smaller helper functions that are specialized and nicely encapsulate their task. One final note, they will be exposed in the dev docs portal, which I'm fine with.

Yeah, I think that's perfectly okay -- the functions are now generic enough that they might even come in handy for different purposes.

[ockham]

Committed to Core in https://core.trac.wordpress.org/changeset/56618.