Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
I finally decided that it's best to remove all ?> from the end of all files. I set this to ready to be commited as there is not a single line of code which is modified and we have debated this to death on the devel list.
TODO: update the coding style guidelines.
Comment | File | Size | Author |
---|---|---|---|
#3 | leftovers.patch | 1.77 KB | chx |
no_question_more_at_the_end.patch | 20.22 KB | chx | |
Comments
Comment #1
Uwe Hermann CreditAttribution: Uwe Hermann commentedPatch applies, seems to work (a quick check trying various things on a test-site succeeded).
Comment #2
Dries CreditAttribution: Dries commentedCommitted to HEAD. Marking this 'active' until the coding guidelines have been updated.
Comment #3
chx CreditAttribution: chx commentedI have not patched *.php so that templates won't get hurt. But this meant a few files are left out... here.
Comment #4
Thomas Ilsche CreditAttribution: Thomas Ilsche commentedWhat do you think about leaving a comment at the end of every file like
// end tag intentionally omited
should ease the entrance to new users, prevent confusion, issues...
Comment #5
Uwe Hermann CreditAttribution: Uwe Hermann commented+1. This will prevent lots of questions in the forums...
Comment #6
Morbus IffNo, it won't. It'll just turn "why is the thing missing" to "so, uh, what's the reason it''s missing?" Intent has nothing to do with explanation. If we include a comment like this, we should include a page in the docs explaining exactly why, and then include that URL in the comment. Only then will it pre-empt questions.
Comment #7
webchickEven better would be something like:
Note: 2 "t"s in omitted.
This would stop both the "Hey, you guys forgot to put ?>s at the end of your files!" as well as the "Hey, WHY is the end tag intentionally ommitted?"
The only problem is that's now an extra ~100 bytes X however many core files there are + contrib module files, etc.
I agree that this information definitely needs to be available (and in a very prominent place) in order to address forthcoming questions/concerns, but I'm not sure if appending to the end of each file is necessarily the best way...
Comment #8
kbahey CreditAttribution: kbahey commentedAngela.
I was just thinking the same thing.
A better arpproach would be to summarize the mailing list dicussion to a Drupal.org page in the FAQ section, and point to it.
Comment #9
nedjoAn FAQ addition is a good idea, but I think we can safely document and not append a message. The same message in every file is bloat (unless it's needed for e.g. licensing). People will figure it out.
Comment #10
Morbus IffWell, if we're not gonna have an explanatory message inline, I'd prefer something like:
commented; see http://drupal.org/faq/tricky_code_that_requires_explanation
?>
Note: I'm still not a fan of this ;)
Comment #11
Morbus IffGrr. Well, that screwed up. Sure wish Preview worked ;) Let's try this:
// ?> http://drupal.org/faq/tricky_code_that_requires_explanation
Comment #12
chx CreditAttribution: chx commentedFolks, this is simply silly. Next what, we add a comment to every single SQL query explaining what's {} around the table names? (It's not in the db_query docs, it is in db_prefix_tables.)
Or we begin to pick label some features of the PHP language as "strange" and comment them like no tomorrow? For example the "alternative syntax" in templates? Heredoc syntax (if we use that at all)? Write lengthy essays about what our pregs do just because they are hard to understand? Forget this but please commit my patch :)
Comment #13
Morbus Iffchx: ANY tricky code needs to be commented. If a piece of code makes you go "hmmm...", then yes, it needs to commented. Unlike, however, our internal documentation which can be traced to a solution (for your bracket/SQL example), there is no traceable way to find why the ?> is missing. Sure, I could spend an hour Googling around, just like I could spend a month reading everything about regexps to try and decipher what the hell is going on with an uncommented core regexp. But this is why every language has a commenting feature: to explain tricky code. This code is "tricky" because it makes people ask questions, it is not immediately obvious, nor has it affected every single user out there. Likewise, it's certainly not a popular snippet that can be seen in tons of other pieces of PHP coding.
Likewise, you talk about this like it's some sort of major feature that everyone should be aware of. If we go back to Gabor's page on php.net (http://us3.php.net/basic-syntax.instruction-separation), you'll notice that it is merely a "Note". If this were really a full-fledged feature, as opposed to just "something you could do", it'd be a lot more than a "Note". A "Note" is something you say "Oh, yeah, you could also...", a side effect, a clarification of something else entirely.
And finally, Gabor's page, assuming people do find it by browsing through Google, doesn't even explain why WE, Drupal, are choosing to do this - it's an off the cuff "oh, yeah, it's optional, and occasionally useful when you use includes". The impression people will get is that it's an optional stylistic trick, NOT that it is actually fixing a bug. Because of this, it'll become equivalent as check_markup: few third party modules will actually use it because no one understands WHY they should.
Comment #14
Dries CreditAttribution: Dries commentedComment #15
Gábor HojtsyMorbus, your points are mostly valid, but the solution is not to add bloat to every file. We have some coding guidelines. That states where should parenthesis go, where you should use two spaces, alternative control structure syntax, etc. Omitting of the closing tags is a similar matter, plus it solves issues. It should be documented in the coding guidelines. If people find this pattern, they will (at least should) look into the guidelines. It is not that any piece of the guidelines should be included in every file.
Watch out for my "omitting T_CLOSE_TAG" thread on php.internals, I hope it is going to catch on, so I can clarify the PHP documentation.
Comment #16
Gábor HojtsyComment #17
kbahey CreditAttribution: kbahey commentedHere is an example of how much confusion this can cause:
In issue http://drupal.org/node/30054 the patch attached has at the end:
That bit in German is automatically added (no newline at end of file?), and is not the issue.
The important point is that the user had to edit the file and add the closing tag, thinking it is missing.
Comment #18
Gábor Hojtsykbahey, that -/+ does not mean he added the "missing" closing tag, it means it wsa there already (or therwise the minus would not be there), but his editor automatically removed the trailing newline at the end of file. This is typical.
Comment #19
(not verified) CreditAttribution: commentedComment #20
(not verified) CreditAttribution: commentedComment #21
ax CreditAttribution: ax commentedmarking ACTIVE again as the
apparently hasn't been done yet.
(just for the record: i didn't follow this issue, and i was *quite* confused after i cvs-updated my Drupal and found many ?>s missing. did someone mess with the repository? strange line endings issues? ...
all in all: loss of some work hours, and, imo, quite a strange move)
Comment #22
webchickI've updated both CODING_STANDARDS.html in the contrib repository, and added a note to http://drupal.org/node/545 (PHP Code tags). The Coding Standards handbook page should be updated shortly, as soon as Dries can get to it.
Comment #23
(not verified) CreditAttribution: commented