I was / am a new user of phpTemplate and Drupal as a whole. I felt that page.tpl.php was not very well commented, so I went in and added reasonable comments and spacing. I think this improves the readability of the code, at a small file size increase. Also, indentation was re-done in tabs.

This is probably the most important and highly edited template for phpTemplate users, so extra commenting couldn't hurt. -jetsetter

CommentFileSizeAuthor
page.tpl_6.php3.43 KBjetsetter

Comments

morbus iff’s picture

Status: Needs review » Needs work

-1. Drupal spacing standard is not to use tabs, but two spaces. Likewise, all your HTML comments are being output to the user, each and every time, which is just a waste of bandwidth - if anything, I'd prefer them to be PHP comments (so that they magically go away come parse time). In some cases, too, you use "

" is absolutely demonic and should not be in there. "Index Page" is not a good alt tag either, IMO - that's naming it based off the technology (the Apache DirectoryIndex) and not the purpose (the front, or main, page of the site).

An honest first effort, but needs some work (and, ultimately, a patch file).

morbus iff’s picture

Project: PHPTemplate » Drupal core
Version: 4.6.x-1.x-dev » x.y.z
Component: Template Defaults » theme system
Geary’s picture

Can't jetsetter get a +1 for trying to help, even if it turns out to be a -1 on the details? Let's encourage people who are new to Drupal and want to contribute. Thanks!

morbus iff’s picture

Actually, it appears my comment was cutoff, and I didn't notice it until now. In any case, I did include words to that effect ;)

jetsetter’s picture

Encouragement is great, thanks.

I'm not sure why I named this "...inadequate fonts..." fonts? I meant comments. Whatever.

Ok, so if I were to try this again with the correct spacing and used php comments, do you think it would be more helpful?

js

morbus iff’s picture

To finish my comments (which were oddly cutoff) the other things I had misgivings about was that you used <!--WORD and <!-- WORD (ie., some with spaces, some without), and the [br class="clear"] has absolutely no business being in a template. That's horrific. I'd also like to see a patch, as opposed to a replacement file. The [meta] tag for "Content-Style-Type" needs to be removed too. That one makes no sense. And the alt text for "Index Page" needs work - that's using technical lingo (it literally relates to the "Apache DirectoryIndex") as opposed to meaning (the "Front" or "Main" or "Home" page of the site). You also do "print $thing" and "print($thing)" - we don't use parenthesis for print. One of your comments uses "2ndary" which is bad form (rememer, some of are users don't speak English, so would have some difficult first getting this to "secondary" and THEN translating it). Et cetera.

As I intended to say in my first comment, an honest first effort, but needs a lotta work.

jetsetter’s picture

Ah.

Ok. Back to the drawing board on this one. Thanks for going over it. I'm so far beyond where I was when I first authored this that I'll have trouble coming back to it at this point. I'll put it on the to-do though.

Jaza’s picture

Title: page.tpl.php file has inadequate fonts for new users » page.tpl.php file has inadequate explanatory comments
Version: x.y.z » 6.x-dev

page.tpl.php for all the themes in 5.0 core could use more explanatory comments, so moving this issue to 6.x-dev queue and leaving it open.

- patch file needed
- consistent commenting style needed
- mix of HTML and PHP comments (as appropriate - because some comments don't need to be outputted) would be good too IMHO

bdragon’s picture

Status: Needs work » Active

Needs an actual patch.

dvessel’s picture

Status: Active » Fixed
Anonymous’s picture

Status: Fixed » Closed (fixed)