Remove the _doing_it_wrong from WP_Block_Patterns_Registry::unregister()

[johnbillion]

There's a _doing_it_wrong() call inside WP_Block_Patterns_Registry::unregister() when you try to unregister a block pattern that doesn't exist.

IMO this is incorrect usage of _doing_it_wrong() because the function hasn't been incorrectly called, it's just been called with invalid data.

---

In addition, the register() and unregister() functions in this class ought to be returning a WP_Error instead of boolean false. Should we improve this for 5.5?

[youknowriad]

I don't feel strongly personally but this is modeled over existing block style variations and block types registries. If we make a change, we should check that it's consistent with other registries.

[isabel_brison]

Let's remove the _doing_it_wrong() in the Block Patterns Registry for 5.5 as that registry is only just being added, and then remove any other incorrectly used instances later for consistency.

Remove _doing_it_wrong() in WP_Block_Patterns_Registry::register() and WP_Block_Patterns_Registry::unregister() and return WP_Error instead of false when used with incorrect data.

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

[desrosj]

At first I thought since this would change the data type that is returned, it could be a risky change. However, block patterns have not shipped in core yet, so I think this is a fine change to make. Only plugins that are building off the Gutenberg plugin would potentially have an issue. But the chance they are calling unregister() incorrectly is probably slim.

[SergeyBiryukov]

Replying to youknowriad:

I don't feel strongly personally but this is modeled over existing block style variations and block types registries. If we make a change, we should check that it's consistent with other registries.

I agree with that. For reference, here's the list of classes and methods that follow the pattern:

• WP_Block_Pattern_Categories_Registry::(un)register() (introduced in 5.5.0).
• WP_Block_Patterns_Registry::(un)register() (introduced in 5.5.0)
• WP_Block_Styles_Registry::(un)register() (introduced in 5.3.0)
• WP_Block_Type_Registry::(un)register() (introduced in 5.0.0)

While there should be no back compat concerns in changing the former two, there should be an investigation of possible side effects for changing the latter two.

While I agree switching all of these to return WP_Error on failure would make sense, changing only one or two classes would bring an inconsistency without a strong reason. I think being consistent in our APIs outweights the benefits of such a change.

I'd suggest either changing all of these at once, or just going with the existing approach.

[whyisjake]

@donmhico can you update the PR to account for the four cases above?