Brain-WP / Cortex

Routing system for WordPress
MIT License
348 stars 20 forks source link

Fixed get_post() call inside template (see #35) #36

Closed metaline closed 2 years ago

metaline commented 2 years ago

Added handle_404() and register_globals if Cortex is booted, like query_posts was added in #32

Fixed #35

gmazzap commented 2 years ago

Thanks, @metaline!

Question: are you sure we need handle_404()? I think to fix #35 register_globals() should be enough.

That said, could you please use the same code style? Your code looks indented pretty weirdly.

I also think that both this and #32 should be done if the matched rout object is a QueryRoute (the default one) but that would require more work, so let's stick with this for now.

metaline commented 2 years ago

In fact, I wasn't sure about the handle_404() either. I added for completeness with the WP::main(). I removed it for now, if we need it, we will add it again!

gmazzap commented 2 years ago

@metaline Sorry, I was thinking something.

The code looks very fine for me now, but it works well only for WP 6+.

Would you have the time and will to check for that before doing both query_posts and register_globals?

I am thinking something along the lines of:

global $wp_version;
if ( ! $do && $wp_version && version_compare( $wp_version, '6', '>=' ) ) {
   $wp->query_posts();
   $wp->register_globals();
}

Alternatively, we could look into the main query request. If that is filled, posts were queried so we can avoid calling it again.

global $wp_query;
if ( ! $do && (!($wp_query instanceof \WP_Query) || empty($wp_query->request))) {
   $wp->query_posts();
   $wp->register_globals();
}

As I said, later on, I want to introduce a condition to do that only if the current route is a query route, but this could be easy to implement and avoid performance issues on WP < 6.0.

metaline commented 2 years ago

I like the second solution, but it can't work, because you're still inside the do_parse_request that runs before the WP::query_posts(), and $wp_query is still empty.

For the moment, I think the only solution we have is WordPress version check.

As I said, later on, I want to introduce a condition to do that only if the current route is a query route, but this could be easy to implement and avoid performance issues on WP < 6.0.

Would be great!

gmazzap commented 2 years ago

Thank you very much @metaline