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
| Comment | File | Size | Author |
|---|---|---|---|
| page.tpl_6.php | 3.43 KB | jetsetter |
Comments
Comment #1
morbus iff-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).
Comment #2
morbus iffComment #3
Geary commentedCan'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!
Comment #4
morbus iffActually, it appears my comment was cutoff, and I didn't notice it until now. In any case, I did include words to that effect ;)
Comment #5
jetsetter commentedEncouragement 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
Comment #6
morbus iffTo 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.
Comment #7
jetsetter commentedAh.
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.
Comment #8
Jaza commentedpage.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
Comment #9
bdragon commentedNeeds an actual patch.
Comment #10
dvessel commentedhttp://drupal.org/node/163723
cleaning up.
Comment #11
(not verified) commented