Code cleanup #1178
Code cleanup #1178
Conversation
A lot of whitespace cleanup and some other inconsistencies in formatting with regard to WordPress PHP coding standard.
comments.php
Outdated
| @@ -38,7 +38,7 @@ | |||
|
|
|||
| ?> | |||
|
|
|||
| </section> | |||
| </section> | |||
There was a problem hiding this comment.
Looks like there is an extra space after the tab on this line.
There was a problem hiding this comment.
Good catch! It was an extra space before, but that should have been removed.
Btw, when I look at this file the section starts with this at line 15:
<section id="comments"><?php
so to me we could as well change the ending from:
?>
</section>
to:
?></section>
for consistency.
(Also, there are two blank lines at line 16 and 17, one of them should be removed as well.)
There was a problem hiding this comment.
(As a side note, CodeSniffer suggested that all <?php and ?> should be placed on their own lines but that seemed too radical to me, so I discarded all those suggestions.)
There was a problem hiding this comment.
I'm kind of torn on this one. On one hand I don't really like how it looks starting a line with a closing php tag (?></section>) and having stuff come after it on the same line. On the other hand it uses up more space to have the php tags on their own lines.
Either way, the opening and closing tags should be consistent so one of them should be moved. The codesniffer didn't say anything about the opening or closing tag here?
There was a problem hiding this comment.
The codesniffer didn't say anything about the opening or closing tag here?
Yes, it did. CodeSniffer suggested that all should be placed on their own lines but that seemed too radical to me, so I discarded all those suggestions for now. But looking at the section for opening and closing php tags in the WordPress php code standard it is actually very clear:
When embedding multi-line PHP snippets within a HTML block, the PHP open and close tags must be on a line by themselves.
It's just that we have a lot code like <?php while ( have_posts() ) : the_post(); ?> that CodeSniffer was breaking up in multiple lines and I wasn't sure we were ready for that.
There was a problem hiding this comment.
@Aetles I think the rule should be if the opening and closing php tags appear on the same line then it's ok, otherwise put them on their own line.
I looked over the PR again and I think if you just remove the space on line 41 and put the opening php tag on line 15 on its own line, we are good to merge.
| @@ -13,7 +13,7 @@ | |||
| if ( ! class_exists( 'Foundationpress_Mobile_Walker' ) ) : | |||
| class Foundationpress_Mobile_Walker extends Walker_Nav_Menu { | |||
| function start_lvl( &$output, $depth = 0, $args = array() ) { | |||
| $indent = str_repeat("\t", $depth); | |||
| $indent = str_repeat( "\t", $depth ); | |||
There was a problem hiding this comment.
Why is there 2 spaces before the equals sign here? Is this so that it lines up with the equals sign on the line below?
There was a problem hiding this comment.
Yes, just like the change here where it's more obvious. This one though is a little odd because the next line has a dot before the equal sign so they only line up visually, but I thought it maybe was within the standard, so I let it in. It was one of those cases where I could go either way.
There was a problem hiding this comment.
@Aetles if that's what the codesniffer said then I'm fine with it. I just wanted clarification and the other example you showed definitely confirms that they are lining up the equal signs. Thanks!
Also remove to unnecessary blank lines.
|
@colin-marshall I've removed the space, som blank lines and moved the opening php tag to it's own line in ´comments.php´. |
A lot of whitespace cleanup and some other inconsistencies in formatting with regard to WordPress PHP coding standard.
This continues the work of #1172 where other similar cleanup was made. I ran FoundationPress through WordPress-Core standards in Codesniffer but did not commit every change that was made, instead I hand-picked the ones that made most sense. Some of the files are formatted due to Foundation and files like kitchen-sink.php is not relevant so I discarded changes there.
Before accepting this pull request, look at the diffs! Some of the changes might be too much depending on taste and opinion.