Skip to content
This repository has been archived by the owner on Sep 16, 2019. It is now read-only.

Consistent Code Indentation/Formatting Across PHP Files #1172

Closed
giollianosulit opened this issue Dec 1, 2017 · 7 comments
Closed

Consistent Code Indentation/Formatting Across PHP Files #1172

giollianosulit opened this issue Dec 1, 2017 · 7 comments

Comments

@giollianosulit
Copy link

Looks like a few php files may have inconsistent indentation/formatting.

Examples:

  • 404.php
  • front.php
  • kitchen-sink.php
  • mobile-off-canvas.php
  • mobile-top-bar.php
@JPOak
Copy link
Contributor

JPOak commented Dec 2, 2017

Kind of in connection to this, is there a reason for the single extra space in front of things like get_header, main-container, and get_footer etc? This could throw tabbing off from the get go. I see this in page.php as well as others.

@colin-marshall
Copy link
Collaborator

colin-marshall commented Dec 2, 2017

@JPOak no reason that I know.

Also in relation to this topic, I think it might be helpful to add comments following closing div tags that say what tag is being closed. So page.php would look something like this:

<?php
get_header(); ?>

<?php get_template_part( 'template-parts/featured-image' ); ?>
<div class="main-container">
    <div class="main-grid">
        <main class="main-content">
            <?php while ( have_posts() ) : the_post(); ?>
                <?php get_template_part( 'template-parts/content', 'page' ); ?>
                <?php comments_template(); ?>
            <?php endwhile;?>
        </main><!-- main-content end -->
        <?php get_sidebar(); ?>
    </div><!-- main-grid end -->
</div><!-- main-container end -->
<?php get_footer();

What do you guys think? Putting it on the main element might be unnecessary as that is obvious what tag it's closing. I think it's kind of helpful on the less obvious closing div tags, but it also makes the code a bit "crowded".

@JPOak
Copy link
Contributor

JPOak commented Dec 2, 2017

@colin-marshall I vote for the comments. I agree that it is not needed for main. Noticed another single space before the main-content that is not needed on current version. Your version looks tight. I like not having white space as well, except for after the get_header.

@Aetles
Copy link
Contributor

Aetles commented Dec 5, 2017

Inspired by the massive changes to WordPress core to fix inconsistencies from the coding standard I went through the FoundationPress code with CodeSniffer and hand-picked some changes. Se the PR above. Don't commit this PR without going through the diffs to make sure only desirable changes are made. I was not sure of everything myself.

@Aetles
Copy link
Contributor

Aetles commented Dec 5, 2017

A lot of indentation changes where from this:

if ( ! function_exists( 'foundationpress_function' ) ) :
function foundationpress_function() {
}
endif;

…to this:

if ( ! function_exists( 'foundationpress_function' ) ) :
	function foundationpress_function() {
	}
endif;

…and that touched a lot of lines. I think it's the later is correct but I'm not sure everyone agrees.

@colin-marshall
Copy link
Collaborator

@Aetles I prefer the latter and also believe that is correct.

@kLOsk
Copy link
Collaborator

kLOsk commented Jan 19, 2019

@derweili I think this can be closed too?

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

No branches or pull requests

6 participants