WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 8 months ago

#37173 closed defect (bug) (fixed)

get_the_content() has undocumented globals

Reported by: slimndap Owned by: goranseric
Milestone: 4.7 Priority: normal
Severity: normal Version: 0.71
Component: Posts, Post Types Keywords: good-first-bug has-patch commit
Focuses: docs Cc:

Description

get_the_content() is using several globals that are mentioned, but not explained in the docblock:

 * @global int   $page
 * @global int   $more
 * @global bool  $preview
 * @global array $pages
 * @global int   $multipage

It took me a while before I was able to understand what they are and where/when they are set.

Attachments (3)

37173.patch (1.0 KB) - added by goranseric 11 months ago.
Commented globals
37173.1.patch (1.0 KB) - added by goranseric 11 months ago.
37173.diff (1.1 KB) - added by morganestes 8 months ago.
Updated patch to align global descriptions.

Download all attachments as: .zip

Change History (15)

#1 @slimndap
11 months ago

Same goes for setup_postdata() and WP_Query:: setup_postdata().

#2 @SergeyBiryukov
11 months ago

  • Keywords needs-patch good-first-bug added

@goranseric
11 months ago

Commented globals

#3 @goranseric
11 months ago

  • Keywords needs-patch removed

I have tested this and used https://codex.wordpress.org/Global_Variables as reference. First time commiter, so I hope it will be good to go :)

#4 @DrewAPicture
11 months ago

  • Keywords has-patch added
  • Owner set to goranseric
  • Status changed from new to assigned
  • Type changed from enhancement to defect (bug)
  • Version changed from trunk to 0.71

@goranseric Thanks for the patch, I'll take a look and let you know if any changes are needed.

Assigning the ticket to mark the good-first-bug as "claimed".

#5 @DrewAPicture
11 months ago

@goranseric In the case of descriptions for globals, we should be describing what the global represents at the initial point of access, rather than trying to be specific about what purpose they serve in the current context.

So for $multipage, for instance, it's truly an indicator for whether pagination is play. It's technically an integer, but treated as a boolean.

So a possible description for $multipage might be something like:

 * @global int $multipage Boolean indicator for whether multiple pages are in play.

In the case of $pages, what it really represents is an array of all pages. And so on with the others.

Last edited 11 months ago by DrewAPicture (previous) (diff)

#6 @goranseric
11 months ago

@DrewAPicture thanks for pointing this out to me, and thanks for the example.
I have attached new descriptions according to your comments and also made comments more consistent to others in core.

Nice to see that we have a little freedom in expressing , I went quite stuffy :-D

This ticket was mentioned in Slack in #docs by morganestes. View the logs.


8 months ago

@morganestes
8 months ago

Updated patch to align global descriptions.

#8 @morganestes
8 months ago

  • Keywords commit added

#9 @swissspidy
8 months ago

  • Milestone changed from Awaiting Review to 4.7

#10 @slimndap
8 months ago

Would it also be possible to mention at what point these globals are set? Currently, as a developer, it is really hard to debug get_the_content() because it is so hard to understand where these values are coming from.

#11 @swissspidy
8 months ago

I'm not sure about $preview right now, but most of them are set in WP_Query by setup_postdata(). We should get rid of most them though, see #36934.

#12 @swissspidy
8 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38746:

Docs: Document global variables used by get_the_content().

Props goranseric, morganestes.
Fixes #37173.

Note: See TracTickets for help on using tickets.