WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#15773 closed enhancement (wontfix)

Lazy load $wp_query->is_singular

Reported by: scribu Owned by: markjaquith
Milestone: Priority: normal
Severity: normal Version:
Component: Query Keywords: php5 has-patch
Focuses: Cc:

Description (last modified by scribu)

The $is_singular flag is just an alias for $is_post || $is_page || $is_attachment.

Being a variable means we have to also keep it current whenever one of it's components changes.

I propose we use a magic __get() method that would call is_singular() internally.

Attachments (1)

15773.diff (5.3 KB) - added by scribu 3 years ago.

Download all attachments as: .zip

Change History (14)

scribu3 years ago

comment:1 scribu3 years ago

  • Keywords php5 has-patch added

comment:2 gazouteast3 years ago

  • Cc gazouteast added

Scribu

What about cross-format variables, as opposed to typical blog usage?

e.g. CMS pre-structuring where post, page, attachment ID are all X (the same number) because they're all the same subject, and X is then used as a default value for custom function/taxonomy or whatever?

I can understand why you'd want to enforce use of individualised string variables, but why deprecate a useful "collective" one?

If WordPress is pushing for recognition as a serious CMS alternative to the likes of Drupal and Joomla! then it needs to keep CMS-useful functionality, would be my two cents worth on it.

Or am I missing something here (as usual)?

comment:3 scribu3 years ago

  • Description modified (diff)
  • Summary changed from Drop $is_singular to Replace $is_singular with is_singular()

You are missing something, yeah, but it's my fault. Updated ticket for clarity.

comment:4 markjaquith3 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Owner set to markjaquith
  • Status changed from new to reviewing

Once we move to PHP 5, we could leave $is_singular undefined and use a magic method ( __get() ) to calculate that $is_post || $is_page || $is_attachment logic, and without deprecating anything. Or rather, we could discuss it, at least.

comment:5 scribu3 years ago

Yes, that's exactly what 15773.diff does.

comment:6 scribu3 years ago

  • Description modified (diff)
  • Summary changed from Replace $is_singular with is_singular() to Lazy load $wp_query->is_singular

Similar: #17195

comment:7 westi3 years ago

Personally I think magic __get methods are nasty.

Before making this change we need a reason to do it like a significant performance improvement.

Can you run some measurements on this?

comment:8 nacin3 years ago

I don't think the point here is performance; rather, it's a desire for is_singular to always be accurate in case is_post, is_page, or is_attachment changes.

comment:9 scribu3 years ago

What nacin said.

comment:10 nacin3 years ago

I'm also not convinced that's desired behavior. While it may be for is_singular, it probably shouldn't be for is_home, which like is_singular is calculated based on other flags. As is is_archive. And is_date. Where do we stop?

comment:11 scribu3 years ago

In the case of is_home, I know I've had to manually set it to false in some cases.

Although it's set based on other flags, it's not really an aggregate like is_singular.

But anyway, point taken.

comment:12 westi3 years ago

Also this change is akin to making $is_singular private.

i.e. it is changing the existing API that a plugin could be using and providing no way for it to continue functioning.

Therefore we need a strong case - like performance (I don't think maintainability is one here because we shouldn't be rewriting or significantly changing this code often).

comment:13 scribu3 years ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from reviewing to closed

Pretty sure a performance increase is out of the question.

Closing.

Note: See TracTickets for help on using tickets.