Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#37173 closed defect (bug) (fixed)

get_the_content() has undocumented globals

Reported by: slimndap's profile slimndap Owned by: goranseric's profile 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 9 years ago.
Commented globals
37173.1.patch (1.0 KB) - added by goranseric 9 years ago.
37173.diff (1.1 KB) - added by morganestes 9 years ago.
Updated patch to align global descriptions.

Download all attachments as: .zip

Change History (15)

#1 @slimndap
9 years ago

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

#2 @SergeyBiryukov
9 years ago

  • Keywords needs-patch good-first-bug added

@goranseric
9 years ago

Commented globals

#3 @goranseric
9 years 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
9 years 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
9 years 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 9 years ago by DrewAPicture (previous) (diff)

@goranseric
9 years ago

#6 @goranseric
9 years 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.


9 years ago

@morganestes
9 years ago

Updated patch to align global descriptions.

#8 @morganestes
9 years ago

  • Keywords commit added

#9 @swissspidy
9 years ago

  • Milestone changed from Awaiting Review to 4.7

#10 @slimndap
9 years 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
9 years 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
9 years 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.