Make WordPress Core

Opened 5 years ago

Last modified 3 years ago

#47616 accepted enhancement

Enhancement: doing_shortcode() function similar to doing_filter()

Reported by: keraweb's profile keraweb Owned by: audrasjb's profile audrasjb
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Shortcodes Keywords: has-patch needs-docs needs-dev-note 2nd-opinion needs-unit-tests
Focuses: Cc:

Description

Currently there is no way to determine whether the current code is run from a shortcode callback.
Similar to actions and filters it would be nice to have a doing_shortcode() function.

My idea is to have an optional parameter for the shortcode tag.
If the parameter is passed it will check if that exact shortcode is running.
If no parameter is passed it will return true if any shortcode is running.

Though I believe it's not officially supported, if shortcodes are triggered within shortcodes it would be best to keep an array of current shortcodes and only remove an active shortcode tag if the callback is finished.

Attachments (3)

47616.patch (2.9 KB) - added by keraweb 5 years ago.
Add doing_shortcode() and current_shortcode() functions
47616.1.diff (3.1 KB) - added by audrasjb 4 years ago.
47616-tested-shortcode.png (359.1 KB) - added by hellofromTonya 4 years ago.
Adding shortcode into a post. Results: works as expected after applying 47616.1.diff.

Download all attachments as: .zip

Change History (22)

@keraweb
5 years ago

Add doing_shortcode() and current_shortcode() functions

#1 @keraweb
5 years ago

  • Keywords has-patch added

#2 @afercia
5 years ago

  • Focuses accessibility removed

Removing the accessibility focus, as seems to me this issue is not related to web content accessibility. Please do feel free to re-add it if I'm missing something.

#3 @johnjamesjacoby
4 years ago

  • Keywords needs-refresh added

I am +1 to this idea.

bbPress would use it for theme compatibility, specifically in bbp_get_topics_pagination_base() where the ability to know if it was called inside a shortcode would be very useful.

cc @aaroncampbell who is listed as a maintainer for the Shortcodes component.

#4 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.7

#5 @audrasjb
4 years ago

  • Keywords needs-testing added
  • Owner set to audrasjb
  • Status changed from new to accepted

I agree, it would be particularly helpful.
Refreshed patch and full testing coming :)

@audrasjb
4 years ago

#6 @audrasjb
4 years ago

  • Keywords needs-docs needs-dev-note added; needs-refresh needs-testing removed

The above patch refreshes the previous one against trunk and fixes few docblock issues.

Here is a small procedure for testing both doing_shortcode and current_shortcode new functions

In your theme, declare the following shortcode / callback function:

<?php
function worg_get_testing_foo() {
        if ( doing_shortcode( 'testing-bar' ) ) {
                $return = '[' . current_shortcode() . '] is currently used';
        } elseif ( doing_shortcode( 'testing-foo' ) ) {
                $return = '[' . current_shortcode() . '] is currently used';
        } else {
                $return = 'Not called from a shortcode';
        }
        return $return;
}

function worg_testing_foo_shortcode( $atts ) {
    return 'Shortcode? ' . worg_get_testing_foo();
}
add_shortcode( 'testing-foo', 'worg_testing_foo_shortcode' );

In the content of a post, I added the following shortcode: [testing-foo].
Which returns: Shortcode? [testing-foo] is currently used


In my theme single.php file, I added the following PHP snippet:
echo 'Shortcode? ' . worg_get_testing_foo();

Which returns: Shortcode? Not called from a shortcode


Conclusion

It really works fine! I can imagine plenty of implementation use cases for this ♥️
Adding needs-dev-note workflow keyword.

Last edited 4 years ago by audrasjb (previous) (diff)

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

@hellofromTonya
4 years ago

Adding shortcode into a post. Results: works as expected after applying 47616.1.diff.

#8 @hellofromTonya
4 years ago

  • Keywords commit added

Latest patch works as expected. Here is the code I used for the attached screenshot above:

<?php
function worg_get_testing_foo() {
        if ( doing_shortcode( 'testing-foo' ) ) {
                return 'Yes. [' . current_shortcode() . '] is currently used';
        }

        return 'No. Not called from a shortcode';
}

add_shortcode(
        'testing-foo',
        function () {
                return '<p>Doing shortcode? ' . worg_get_testing_foo() . '</p>';
        }
);

Marking for commit consideration.

#9 @jorbin
4 years ago

  • Keywords 2nd-opinion added

In theory, I like this enhancement, but I'm less sure about the implementation.

I have some concerns about the proposed implementation of current_shortcode. Since shortcodes can be nested, it's possible to be in multiple shortcodes at once yet checking current_shortcode() will only return one. This can lead to unexpected behavior.

I also don't love the idea of adding yet another global variable.

#10 @jorbin
4 years ago

  • Keywords needs-unit-tests added; commit removed

Also, noting that the lack of tests means this isn't ready for commit.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

#12 @hellofromTonya
4 years ago

  • Milestone changed from 5.7 to 5.8

@jorbin raises valid concerns that need consideration. 5.7 Beta 1 is < 4 hours. Punting this ticket to 5.8 to give more time.

#13 @johnjamesjacoby
4 years ago

Ran into another instance where this would have been useful.

Since shortcodes can be nested, it's possible to be in multiple shortcodes at once yet checking current_shortcode() will only return one. This can lead to unexpected behavior.

Multisite and switch_to_blog() has/had a similar implementation concern, where it's "stack" of switched sites (in the $GLOBALS['_wp_switched_stack'] global) has functions to navigate it.

In the case of shortcodes, I would imagine that current_shortcode() should always return the one that is being executed right now, inside an array or stack of the currently nested shortcode calls. That would make doing_shortcode() work similarly to ms_is_switched(), which it mostly appears to in the patch already – doing any, or doing one.

Maybe doing_shortcode() could include a second parameter for if it's the right now shortcode or the anywhere in the stack shortcode.

Perhaps the $wp_current_shortcode global should be renamed to $_wp_shortcode_stack to be consistent with others? I'm not fluent with the current core naming guidelines, but I'm just bringing it up since I'm talking about multisite above.

#14 @keraweb
4 years ago

@jorbin @johnjamesjacoby
I fully agree with your idea's and concerns, especially the new global.
However, if you would like to change this we should change it for the actions as well since they work exactly the same. That is in fact the reason why I've created the patch this way; consistency.
Preferably I'd create a new class for functionality like this and let the actions/filters and shortcodes extend it.

#15 @desrosj
3 years ago

  • Milestone changed from 5.8 to 5.9

It seems this one needs a bit more time for refinement. I'm going to punt to 5.9 as there's a bit of momentum. But it needs someone to revisit.

#16 @keraweb
3 years ago

If this is preferable I can make a class for this. But in that case I would like to include actions etc. as well (in separate classes, extending the same base). We already have WP_Hook where we can store the current action etc. but for shortcodes there isn't anything OOP we can use at the moment so this will need to be created.

I'd like to have some more feedback on this before I continue!

#17 @audrasjb
3 years ago

  • Milestone changed from 5.9 to Future Release

Since tomorrow is WP 5.9 feature freeze, I'm moving this ticket to Future release.

#18 @keraweb
3 years ago

@audrasjb Why not 6.0? Also, I wouldn't call this a new feature btw, it's an enhancement.

While the ticket has become silent I'm still waiting for feedback.
I'd like to continue on this but without feedback on how to proceed I'm not sure what to do.

The patch I provided already works and is written the same way as the current doing_filter() implementation.

I'd hate to see this ticket to be forgotten entirely in the Future Release tag as so many others..

#19 @audrasjb
3 years ago

@keraweb 6.0 doesn't exists for now in Trac :)
I'll ask Meta to create the milestone, so we can move all the tickets that already have a patch and just need review/refresh in 6.0.

Note: See TracTickets for help on using tickets.