Reverse wrapping of `apply_shortcodes()` and `do_shortcode()`

[SergeyBiryukov]

Background: #37422, #55852.

In WordPress 5.4, ​apply_shortcodes() was added as an alias for do_shortcode() for better semantics. It aligns more with apply_filters() and the semantics of applying filters to the input and returning a result, rather than performing an action and outputting to the current buffer.

Since we've revisited sanitize_url() and esc_url_raw() in [53452], I think it's time to consider doing the same here. Should we make apply_shortcodes() the recommended function, instead of do_shortcode()?

[rafiahmedd]

@SergeyBiryukov should we make the changes in wp-includes/shortcodes.php?

[SergeyBiryukov]

Replying to rafiahmedd:

should we make the changes in wp-includes/shortcodes.php?

If there is enough consensus for this to go forward, then yes, that would be the start :) The next step would be replacing all do_shortcode() calls in core with apply_shortcodes().

[namith.jawahar]

Seems like the logical thing to do, What about related functions like do_shortcodes_in_html_tags and filters like pre_do_shortcode_tag

[rafiahmedd]

Replying to SergeyBiryukov:

If there is enough consensus for this to go forward, then yes, that would be the start :) The next step would be replacing all do_shortcode() calls in core with apply_shortcodes().

Then I think we have to wait for some more commiters to make a decision about this. And I can start replacing this from now.

[SergeyBiryukov]

Replying to namith.jawahar:

What about related functions like do_shortcodes_in_html_tags and filters like pre_do_shortcode_tag

Good question, thanks! For consistency, I think we can use the new naming for them as well.

Functions:

• do_shortcode_tag() → apply_shortcode_tag(). Since the original function is marked as private, we can probably deprecate it in favor of the new one. Looking at the ​usage in plugins, most instances appear to be false positives or copies of the function.
• do_shortcodes_in_html_tags() → apply_shortcodes_in_html_tags(). The original function is not marked as private and is ​used in about 15 plugins. It can be up for discussion whether to deprecate it too, or keep as an alias of the new one.

Filters:

• pre_do_shortcode_tag → pre_apply_shortcode_tag. ​Used in about 25 plugins.
• do_shortcode_tag → apply_shortcode_tag. ​Used in about 132 plugins. Looks like we'll have to keep the old filters as aliases for a while, though using apply_filters_deprecated() for them can also be an option.

As a side note, we might want to do the same for do_blocks() → apply_blocks() in a separate ticket.

[rafiahmedd]

@SergeyBiryukov should we start replacing these?

[peterwilsoncc]

@SergeyBiryukov I endorse this change.

Looking at WordPress.org, it looks like various pages of docs will need updating but nothing major. ​https://www.google.com/search?q=site%3Awordpress.org%2F+do_shortcode+-topic

This PR changes the wrapping of apply_shortcodes() and do_shortcode() such that apply_shortcodes() is now the recommended function.

In addition:

• Calls to do_shortcode() have been changed to apply_shortcodes().
• Some default filter callbacks have been changed from 'do_shortcode' to 'apply_shortcodes'.
• Applicable documentation has been updated to refer to apply_shortcodes() instead. (please check these!)

This PR does not touch:

• Any other do_shortcode_*() function, as I'm not clear if a decision has been made about introducing new wrappers or deprecating the do_shortcode_*() variants.
• Any hooks, as I'm not clear if a decision has been made about keeping the hooks, or using apply_filters_deprecated().
• ​The default filter callback for widget_text_content, as this causes test failures and may impact legacy support.
  • See ​here and ​here.
• ​This line in wp-includes/blocks/template-part.php as this file is sourced from Gutenberg.

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

[audrasjb]

Looks go to me!
Let's start with this first implementation for 6.1, then we can open a new ticket to address filters, and open an issue upstream on Gutenberg repository to address the occurrence located in Gutenberg.

Self assigning to commit this PR.

[audrasjb]

In 54248:

Shortcodes: Reverse wrapping of apply_shortcodes() and do_shortcode().

This changeset reverses the wrapping of apply_shortcodes() and do_shortcode() such that apply_shortcodes() is now the recommended function. In addition:

• Calls to do_shortcode() have been changed to apply_shortcodes().
• Some default filter callbacks have been changed from 'do_shortcode' to 'apply_shortcodes'.
• Applicable documentation has been updated to refer to apply_shortcodes() instead.

Follow-up to [47004].

Props SergeyBiryukov, rafiahmedd, namithjawahar, peterwilsoncc, costdev.
Fixes #55883.

Committed in https://core.trac.wordpress.org/changeset/54248

[ocean90]

[54248] will break any usage of remove_filter( 'the_content', 'do_shortcode', 11 );. How can this be avoided?

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

[davidbaumwald]

Prepping a commit to revert the filter changes.

[davidbaumwald]

In 54278:

Shortcodes: Revert default filter callback changes from apply_shortcodes to do_shortcode.

[54248] reversed the wrapping of do_shortcode and apply_shortcodes and updated all direct internal calls of do_shortcode to apply_shortcodes after [47004]. Default filter callbacks that used do_shortcode were also updated to use apply_shortcodes. However, this introduced a backward-compatibility break because any attempt to unhook a filter using the previous do_shortcode callback would be futile.

This change reverts only the filter callback changes in [54248] to resolve the backward-compatibility break.

Follow-up to [47004] and [54248].

Props ocean90, SergeyBiryukov.
See #55883.

Merged into core in https://core.trac.wordpress.org/changeset/54278.

[TobiasBg]

@davidbaumwald: Should an inline comment be added to these lines, so that caution is taken once do_shortcode is formally deprecated, or so that this change is not repeated accidentally?

[davidbaumwald]

@TobiasBg given its age and widespread usage, I don't think there's a plan to deprecate do_shortcode. I think extra care should just be taken when changing default filter/function names in the future.

[azaozz]

I'm sorry but I don't think this is a good change.

Is apply_shortcodes() really that much better (in plain English) than do_shortcode(). Is it really worth it breaking all references to do_shortcode just for the sake of changing do with apply? I think not.

do_shortcode has existed for many many years. It is referenced in a lot of places, online documentation, many posts, thousands of themes and plugins, many books, etc. Why would WordPress want to break all these references? It is the same as breaking links, very bad UX.

I think it is a pretty bad idea to create all that confusion and broken references for such a minor benefit. A much better solution imho would be to update the docs, add as much content there as needed. That will serve all existing WP contributors and help all new contributors, and most importantly, will not break any links and cause confusion.

Also I'm not sure if the apply_shortcodes alias was needed in the first place. It's been around for couple of years, since WP 5.4, but seems the adoption is pretty low? Think it can be classified as an "unsuccessful experiment" at this point. A quick search shows 134 matches in plugins for apply_shortcodes versus 38653 matches for do_shortcode. See ​https://wpdirectory.net/search/01GDK9ZK4WX0KQT21ZS28K9E06 and ​https://wpdirectory.net/search/01GDKA13MP0C6FKDYXWHPPXQ9P. That alias probably just brings some confusion for new contributors.

I'd like to propose that this is reverted and closed. This type of name changes is not worth the trouble and confusion they bring imho.

[JeffPaul]

@azaozz I think the revert you're requesting was handled in r54278, correct?

[azaozz]

Replying to JeffPaul:

No, [54278] was a partial revert to fix a regression. What I meant is to revert the whole "rename" of do_shortcode() to apply_shortcodes() as committed in [54248]. Imho it is not good to break all existing references (38k in plugins and about 263k on Google) to the old name, as it is not that bad. The confusion of these broken references far outweighs any possible benefits of renaming. The partial revert seems to have made this even more confusing.

Another reason for full revert would be that "apply shortcodes" doesn't describe properly what that function does. It does search and replace, and in English you usually cannot say "apply search and replace". The proper way to say that would be to "do search and replace". Maybe not such a big deal, but why not use good English :)

[jorbin]

I agree with @azaozz that there is little to no benefit in renaming this function and we should revert the renaming.

[davidbaumwald]

Prepping a full revert for this now.

[davidbaumwald]

In 54319:

Shortcodes: Revert recent apply_shortcodes and do_shortcode changes.

[54248] reversed the wrapping of do_shortcode and apply_shortcodes and updated all direct internal calls of do_shortcode to apply_shortcodes after [47004]. After further consideration, the long history of do_shortcodes should be favored over any subjective semantic improvements. This change reverts the remaining changes from #55883 not already reverted in [54278].

Follow-up to [47004], [54248], and [54278].

Props azaozz, jorbin.
See #55883.

[davidbaumwald]

Cleaning up the tags a bit post-revert.

[audrasjb]

Even if I weighted in by committing the initial patch (since it was working, reviewed, and nobody raised any issue with the change), I can see how this change is probably a bit overkill. Thanks for sharing a detailed review on this 👍

[TobiasBg]

I agree that reverting this is the proper decision.

While the initial idea of introducing the alias and potentially considering deprecating do_shortcode() if the alias gained enough traction, was definitely worthwhile, the problem is that [54278] will always be needed:
If Core were to change add_filter( 'the_content', 'do_shortcode', 11 ); to the alias, there would be an immediate back-compat break. First deprecating do_shortcode() (with a proper _deprecated_function() call in it) would mean that Core itself would be triggering these warnings. Soft-deprecating do_shortcode() without a _deprecated_function() call (basically the status quo after the revert, and from wording of initial commit messages and the 5.4 dev note) has not resulted in widespread use of the alias (see azaozz's posted links).

Due to there not being a feasible path to fully switching to a new function name, the rich history and widespread use of do_shortcode(), the low use of apply_shortcodes(), I would therefore actually suggest to deprecate apply_shortcodes() again. There's no need to keep a rarely used alias that Core itself will not be able to use and that plugin developers have not started to use in 6 or 7 WP releases. I agree with azaozz that this was an experiment that simply has shown to be unsuccessful, due to lack of user adoption and (only now) brought-up BC concerns.

[azaozz]

Replying to audrasjb:

Sorry it took so long to review this. At first I thought having more aliases is a good idea, a chance to improve some function names without breaking back-compat. But thinking more about it the price to pay for that is usually pretty high considering all the broken references to the old names. Still think is would be good to add aliases for some of the old functions where (over the years) the code and usage have moved beyond the initial intent. If nothing else having an alias would make the "true use" easier to discover. Not sure about replacing the names that are used in core. Seems that may get confusing.

[azaozz]

Replying to TobiasBg:

Due to there not being a feasible path to fully switching to a new function name, the rich history and widespread use of do_shortcode(), the low use of apply_shortcodes(), I would therefore actually suggest to deprecate apply_shortcodes() again. There's no need to keep a rarely used alias that Core itself will not be able to use and that plugin developers have not started to use in 6 or 7 WP releases.

Deprecating apply_shortcodes() sounds like the next logical step. But even if it is deprecated it cannot ever be removed. So perhaps the docs can be updated a bit and it can remain as an alias. Extenders would still have the choice to use it (that's the case even if it is deprecated). However the chances to confuse new extenders and contributors will be super low.

[azaozz]

Imho this is completed now. Leaving it open only as a reminder to perhaps update the apply_shortcodes() docblock a bit before 6.1. Thanks everybody for the great discussion here! :)

[audrasjb]

Closing this as fixed, since 6.1 RC1 is approaching, so we have cleared milestone.
Better to open a new ticket to handle follow-up docblock changes if needed.