Templates: clean up task

[franz-josef-kaiser]

All the templates packaged with the plugins were in terrible shape: Mixed Tabs/Spaces, closing PHP tags at the end of the files, endless line length that needed horizontal scrolling on GitHub, etc.

The pull request focuses on code readability and code structure that allows easier reading and understanding of HTML element nesting. Cleaned up all Tabs/Spaces, etc. Added missing ids to match the labels and what-not-else.

I'm pretty sure that lots of the edges could be ironed out with simply reworking the if/else statements. This would as well flatten the structure allowing for longer lines. Overall this is just a starting point and should serve as base for further improvements.

[franz-josef-kaiser]

So. After I saw the Travis CI Unit Test results and seeing this and this test failing, I looked up the source of EDD_Unit_Tests\Tests_AJAX::test_opt_into_local_taxes and ... man! One can't test a HTML string in a unit test. A single changed space, intend, etc. will let it fail. This whole task is pretty senseless as it assumes that one would never sacrifice source code styling over code styling. I'd suggest to drop every HTML output related unit test at all.

[franz-josef-kaiser]

1. Removed the class_exists() check. EDD is a standalone plugin and the main class isn't meant to be overridden.
2. Removed the plain and lonely EDD(); call in the bootstrap file and hooked the instance generation to plugins_loaded on the (default) priority 10 so child plugins can rely on it. As a long term goal, all those calls to EDD(); should be replaced with Easy_Digital_Downloads::instance() and the function be deprecated at all.

[franz-josef-kaiser]

Ah, yeah - I did test the later two pull request extensions, but the template files should be tested as well. I ain't got shortcodes anywhere.

[pippinsplugins]

While I appreciate the pull, you have completely neglected the WordPress coding standards in your updated template files. We may have some spaces / tabs mixed together inappropriately, but we try (and largely succeed) to follow the WP coding standards. This pull is pretty far off.

If you'd like examples, let me know, I"ll be happy to draw them up :)

[franz-josef-kaiser]

Yeah, please do that. Afaik the WP coding "standards" use Egyptian brackets. N/P reworking it as I got it on the side. :)

The main problem I got with the previous version is the line length. Everything above 80-100 chars per line is unreadable on GitHub and in most IDEs that have a sidebar for e.g. source lookup, etc. It's still not 100%, but it's imo far better. Then there's the problem (and old discussion) with foreach : / endforeach and similar VS. {}. First one might be nicer if you don't understand code, but if you got an IDE - which most people reading source and participating here will have - then you'll have the problem that blocks aren't highlighted. Hence the switch over.

[pippinsplugins]

Here's a quick list:

1. Brackets always go on the same line as the connected statement:

if( $something ) {

Not

if( $something )
{

1. Tabs for indenting.
2. Spaces for aligning arguments and variables

Other notes:

1. Please remove your var_dump() at the top of checkout_cart.php :)
2. I personally don't like the use of AND in files because it's less readable and harder to understand for non-developers.

[franz-josef-kaiser]

Tabs for intending

Don't see any Spaces aside from aligning. Please point me at them if you found any. From a brief check there weren't any.

Remove var_dump()

Ups, done! :)

don't like the use of AND in files because (...) harder to understand for non-developers

Actually there're several reasons for using AND and OR. First, when I started I got around it much easier when I found out about them. Second, there's a difference between || and OR and && and AND - just try it out to see which one overrules the other one. In more advanced, one-case/project-specific code I as well use them instead of additional brackets.

Brackets always go on the same line as the connected statement:

Even if I don't like it, I changed it to WP coding "style" ;)

[pippinsplugins]

I will review and test it as soon as I can. Thanks for the updates!

[spencerfinnell]

While not a standard noted directly, based on the default themes, etc, I think template files should use conditionals like:

<?php if ( $something ) : ?>
<div class="normal-html"></div>
<?php endif; ?>

It feels less "PHPy" and more "templatey" -- And then you don't need comments like // endif and instead you can have comments about what it is actually ending. // empty cart check

[pippinsplugins]

I agree with Spencer. It's easier to read.

[chriscct7]

Agree with Kaiser. Endifs are hard to work with when you've got to find where they end.

[chriscct7]

You don't need endif comments if you use a standard editor with braces. Not sure why you couldn't put "meaningfully comments" either eay

[pippinsplugins]

endif;s are much easier for someone to understand that is not a developer. Braces might as well be gibberish.

[chriscct7]

First off if they are editing a template file either they know PHP (in
which case brace argument negated) or they are getting help doing so in a
copy paste format (brace argument negated again because if they are copy
pasting the whole thing is gibberish).
On Jun 7, 2013 8:56 PM, "Pippin Williamson" notifications@github.com
wrote:

endif;s are much easier for someone to understand that is not a
developer. Braces might as well be gibberish.i9

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1194#issuecomment-19140316
.

[pippinsplugins]

I still disagree. Just because they SHOULD have knowledge of PHP if editing files, doesn't mean they do, and we should consider that and make it as easy as possible for them.

[chriscct7]

And of course, we can always point to the handbook.....no endifs there

If they don't know PHP, likely they'd use an IDE like Visual Studio, or dare I say it, Dreamweaver, both of which can't find the ends of "endif" statements. If anything's easier for them it's using braces. I'd love to find an example of someone who doesn't know PHP AND who is not afraid of editing the files AND is doing it by themselves AND and has no idea what braces are AND is using a resource to learn PHP that uses endifs over braces AND know what templates are AND what they are used for AND where they are found AND how they are used. Odds: Less than getting mauled by a Polar bear, Brown bear, and black bear on the same day. In Guatemala.

If they use any online assistance to help them, say W3CSchools, which alot of beginners do, they use braces.

Then there's project consistency. We use braces everywhere else. Why not here?

At the very least, the 0.0000000001% of users that now fit this criteria, could benefit from learning what braces are. However, isn't the point of templates to help the 99.99999999999% that use them? Like the theme devs, or users with PHP experience who actually even know what templates are, what they are used for, where they are found, and how they are used?

[chriscct7]

We should stick to what we use everywhere else

[pippinsplugins]

Agreed.

[sunnyratilal]

Rather than ! defined( 'ABSPATH' ) AND exit;, I'd prefer defined( 'ABSPATH' ) OR exit;

[franz-josef-kaiser]

Completely agree. That's exactly what I use in every of my own plugins and themes for files that shouldn't get loaded outside the context.

[pippinsplugins]

Template files are the exception to the rule, simply because they are the part that users interact with.

[chriscct7]

Was that a comment to the code comment?

[spencerfinnell]

There aren't any in the handbook yes, but like I said, look at any of the default themes, and all template files use them.

I agree, braces should be used in all other instances, but anything related to the theme, or when PHP and HTML are commingling is an exception in my book

[pippinsplugins]

No, I agreed with the ABSPATH comment.

I"m saying template files are the exception for using endif, endwhile, etc, in stead of brackets.

In order to keep with WordPress standards, I'm requiring that we use endif / endwhile. All of the default WP themes do it that way and it is more semantic for non-developers. The way default WP themes are setup is a lot of user's first experiences with code, so when they are editing simple HTML we should try to adhere to that as much as possible.

Sorry but I"m putting my foot down.

[franz-josef-kaiser]

Sorry but I"m putting my foot down

I can see that.

As two of us participating in this discussion agree and two don't agree, we got a patt situation. So I'd suggest the following, which allows a sane use of the IDE (folding, highlighting, etc.) and still explains the 0.0^10% of those users what it's about:

if ( $condition ) {
    ?>
    <html>Stuff</html>
    <?php
} // end if

foreach ( $foo as $bar ) {
    ?>
    <html><?php echo $bar; ?></html>
    <?php
} // end for each

This imho is much more readable that endforeach; and endif; as it states what it does in words, instead of stuffed together code language (that maybe even scares people away). Which btw is the same reason why I prefer AND and OR over && and || - aside from the specifity/overruling.

[pippinsplugins]

This I am fascinated by: why is {} better than endforeach/endif, but AND / OR is better than && and ||?

They seem like the same thing to me, though I do realize that I don't like AND and OR :P

[chriscct7]

Yeah I don't like AND/OR either.
As for the brace deal, going to open a Trac ticket tonight based on Pippin's blog, this Github post here, and our Core Dev chat. I'll post a link when its up later tonight.

[franz-josef-kaiser]

This I am fascinated by: why is {} better than endforeach/endif,

I already explained it in a comment above:

(..) there's the problem (and old discussion) with foreach : / endforeach and similar VS. {}. First one might be nicer if you don't understand code, but if you got an IDE - which most people reading source and participating here will have - then you'll have the problem that blocks aren't highlighted. Hence the switch over.

Ad the difference between operators

They seem like the same thing to me, though I do realize that I don't like AND and OR :P

No, they are not. I already mentioned it above.

Anyway, I'm not putting my heart into this pull request and the result must please the majority.

[pippinsplugins]

Thanks for taking the time anyway.

[chriscct7]

Trac ticket: http://core.trac.wordpress.org/ticket/24549

[franz-josef-kaiser]

Thanks for taking the time anyway.

No problem.