Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the loop on single.php and page.php appears unnecessary. #301

Closed
drrobotnik opened this issue Aug 26, 2013 · 6 comments
Closed

the loop on single.php and page.php appears unnecessary. #301

drrobotnik opened this issue Aug 26, 2013 · 6 comments

Comments

@drrobotnik
Copy link

Is it for consistency that we include a while conditional around the loop on single/page.php?

https://github.com/Automattic/_s/blob/master/single.php#L13

If there is no post than they'd be redirected to the 404.php so to me it seems unnecessary. Also on a single/page there shouldn't be instances of more than one post without a custom query. I'd change this to an if conditional or remove it entirely like so:

<?php
/**
 * The Template for displaying all single posts.
 *
 * @package _s
 */

get_header(); the_post(); ?>

    <div id="primary" class="content-area">
        <main id="main" class="site-main" role="main">

            <?php get_template_part( 'content', 'single' ); ?>

            <?php _s_content_nav( 'nav-below' ); ?>

            <?php
                // If comments are open or we have at least one comment, load up the comment template
                if ( comments_open() || '0' != get_comments_number() )
                    comments_template();
            ?>

        </main><!-- #main -->
    </div><!-- #primary -->

<?php get_sidebar(); ?>
<?php get_footer(); ?>

Curious on the thought behind the current decision before I submitted a PR.

@kadamwhite
Copy link

have_posts is useful for more than just a conditional check—per the codex:

As a side effect, have_posts starts, steps through, or resets The Loop. At the end of the loop, have_posts returns 0 after calling rewind_posts.

@kadamwhite
Copy link

Hmm, looking at the code behind have_posts, it's actually a fairly straightforward function... I'd say there's value in consistency, though.

@obenland
Copy link
Member

obenland commented Sep 3, 2013

I'm always in favor of removing code where it makes sense. Looking at have_posts(), and the lack of an else statement in the template make me wonder if this could be one of those cases.

@mfields
Copy link
Contributor

mfields commented Sep 3, 2013

I prefer to use the standard Loop in all singular templates in _s. The code suggested above is what we refer to as a "partial loop" and really causes issues in other queries on the page. Historically this has appeared in a couple of the Default themes and has been fixed right away. Please see the following ticket: https://core.trac.wordpress.org/ticket/18794

@obenland
Copy link
Member

obenland commented Sep 3, 2013

Sweet, thanks for the heads up @mfields!

@obenland obenland closed this as completed Sep 3, 2013
@mfields
Copy link
Contributor

mfields commented Sep 3, 2013

No Problem!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants