General Indentation of core-Files

Login or register to post comments
19 replies [Last post]
Offline
Joined: 04/24/2008

Hi folks,

during my work at the Documentation of some of the base-Controls I noticed ... a (at least for me) bogus and unnecessary indentation:

After the opening <?php everything will be tabbed one time. In these files there will never be any HTML nor will (or better should) the tag be closed again. (Not closing it will prevent that trailing whitespace is givven out)

So we could save a lot of space and core-code would be more readable.

What do you say?

RKotenko's picture
Offline
Joined: 07/03/2008

Are you saying that even nested code will not be indented more than one tab?

If so, that is not more readable to me. And saving bytes in the files is not a worthwhile goal to me.

Or maybe you are saying there are places where there are more than the right number of tabs per line?

This:

if(true){
  if(false){
    echo hello;
  }
}

is far more readable than:

if(true){
if(false){
echo hello;
}
}

Not sure what you means about closing the tag either.

Offline
Joined: 04/24/2008

ABSOLUTELY NOT!

What I wanted to suggest is:
Replace:

<?php
   
class SomeClass {
        function
__construct() {
            echo
"It's me, Mario!";
        }
    }
?>

with:

<?php
class SomeClass {
    function
__construct() {
        echo
"It's me, Mario!";
    }
}
?>

without the trailing ?> (left here for activating highlighting)

RKotenko's picture
Offline
Joined: 07/03/2008

Ok, got ya. :)

I don't understand leaving the trailing ?> out. That gets rid of what now?

In any case, I am not sure if this is a worthwhile endeavor. The files will be negligibly smaller, maybe slightly easier to read in some places with nesting and the effort is not worth the advantage. I would be inclined to say no. But I will let others chime in as well.

Offline
Joined: 04/24/2008

Well... effort would be near to zero... I have a script at hand that will remove this leading tab automatically.

To explain why it makes sense leaving out the "?>"

Imagine following situation:

<?php
$intOne
= 1;
$intTwo = 2;
$intThree = 3;
?>

  

Do you see the trailing new line after "?>"? This newline will be send to the browser. If no content-cache (OK, these times we have one per default, but things could change...) is active this will prevent modifying headers, sending cookies, etc.
If the "?>" were left out, the PHP-Interpreter fetches the whitespace and does... yeah! Nothing!
EDIT:
But of course, youre right, saying it's not that important. It isn't indeed. There are many other tasks one should work on. It just came to my mind working with core-classes.

OOPMan's picture
Offline
Joined: 11/07/2008

Please DO NOT remove the trailing ?> tag

Perhaps you are under some conception that it's optional.

Maybe you leave it out in your code.

Possibly you sometimes get weird, inexplicable parser errors.

Maybe you find that adding the closing ?> tag back in removes said errors.

Maybe this has escaped your attention.

It has not escaped mine. Just last week I was working on-site with a developer who happily told me that you can leave out the closing ?> tag. He then began complaining about a certain file not working for no obvious reason. Upon my suggestion he added to trailing ?> tag back in. The file now worked. There were no other erros in the file. I checked.

Well done, noddy badge to all who realise that maybe, just maybe, the idea of WELL FORMEDNESS is NOT complete bollocks.

Please don't screw with the parser. Close your damn PHP tags.

Sorry for the tone of my post, but writing syntactically dangerous code (Even only potentially syntactically dangerous code) PISSES me off.

Offline
Joined: 04/24/2008

Sorry OOPMan, but you are simply wrong! Leaving out the last closing "?>" is officially supported by PHP.

http://de.php.net/manual/en/language.basic-syntax.instruction-separation...

And even the famous Zend-Framework explicitly forbids the closing PHP-Tag:
http://framework.zend.com/manual/en/coding-standard.php-file-formatting....

Edit from 12:33:
Let me quote PHP's arguments here for leaving it out:

Note: The closing tag of a PHP block at the end of a file is optional, and in some cases omitting it is helpful when using include() or require(), so unwanted whitespace will not occur at the end of files, and you will still be able to add headers to the response later. It is also handy if you use output buffering, and would not like to see added unwanted whitespace at the end of the parts generated by the included files.

(find it at above link)

alex94040's picture
Offline
Joined: 11/06/2008

Folks, we've talked about removing the trailing ?> tags before, and no, we're not doing it. Well-formed code is more important than "not noticing" trailing newlines. There are other ways to find / get rid of the trailing newlines.

On the extra tab: yeah, I hear you, this somewhat irritates me, too. It'd be cool to do this for at least the form_drafts and metacontrols... If you feel passionate about doing it, please create a ticket and submit a patch, and we'll review it and include it in.

Open source software is very much about allowing contributors to do what they're passionate about: you seem to be interested in saving everyone time by removing that extra tab... Please go ahead and do so! The community will appreciate your contribution.

Offline
Joined: 03/31/2008

If we're ok with removing the starting tabs, I'll start ensuring any patches I submit include that modification in the future.

That's the default formatting done by my IDE, so it will be nice to be able to use it's built-in formatting support again. :)

Offline
Joined: 07/10/2008

If the line above something is used it should be tabbed.

hence <?php is the first line and the second line is tabbed.

This is obviously preference and does not effect anything.

OOPMan's picture
Offline
Joined: 11/07/2008

fcool, that's all very cute but as I said before, when not closing the tags causes the parser to choke on a file it's not really optional.

Just because one CAN write shoddy, malformed code does not mean that one SHOULD write shoddy malformed code, even if the php developers encourage one to do so.

Frankly, my opinion of the php development team drops every time the push back the release date of php 5.3. It similarly drops when reading the php bugzilla and noting the number of open bugs.

RKotenko's picture
Offline
Joined: 07/03/2008

I agree, Basilieus.

This does not define any sort of standard because it does not make it easier to read. My IDE is set to tab after the <?php. But I don't care if I get a file that is indented or not.

I say leave this up to preference.

Offline
Joined: 04/24/2008

I can live with leaving it, where it is.

What will still simply wrong is, that it would be incorrect code. The PHP Syntax is defined by PHP. And if they say it is optional, it simply is. There is no world standard organisation defining PHP standards, other than the PHP developers (mainly Zend).
It's neither shoddy nor malformed and even the syntax keepers itself suggest it (if there are reasons).

Just look at xml: You can write: <div></div> or <div />. So would any of you say that the latter is shoddy or malformed? Obviously you don't need this content-area in an empty tag. Clever parser!

So the PHP Interpret just interprets the EOF as closing tag. I'm programming PHP since 6 years now, and since i start leaving the tags (don't remember exactly but like to think it was with first publishment of the ZendFramework - Coding - Guidelines) it has never caused any problems for me and my collegues. And i mean NEVER.

But never mind. If it shall stay there, I'm okay with that. For me it's just important to show, that this is just a personal belief, not a fact. There is (and will be) NO problem with leaving it out, and it is PERFECTLY standard conform. (Just remember: we are talking about PHP not HTML. For HTML you would INDEED need a closing tag. But these files does not contain a single line of HTML, that is not wrapped by PHP commands. So it is, as PHP-file, wellformed and valid! For HTML you have to use the closing tag. But even then the file is just wellformed and far away from valid, as there is no <?php - tag defined in HTML and all of the PHP-Documents lack a Doctype, which is needed for HTML-Validity)
But if the community want to keeps it, hey! No problem. It's such a minor thing doesn't worth all the noise. So if we're arguing about "beliefs" (mine, that it would make sense to omit it, others that it looks not as well as before), let's keep in mind, that it ARE beliefs. So just lets be polite and cooperative as ever.

OOPMan's picture
Offline
Joined: 11/07/2008

I know I'm nitpicking, but you line about just look at xml is bogus. According to the XML spec the should be no difference between and . This has got nothing to do with the one being a malformed version of the other and relates to the php tags in no way at all.

As we know, though, some browsers (IE) don't like type empty tags and hence the form is more common.

Offline
Joined: 04/24/2008

You got it! After specification of the W3C (the XML-developers, in some kind of way), there is no difference. (Of course you are right, when it comes to browsers... but that it is another story.)

And so it is with PHP. There are two possibilities to say the same thing: "My code ends here." That is with "?>" or with end of file. And that by specification of the PHP developers.

OOPMan's picture
Offline
Joined: 11/07/2008

I just find it very worrysome that a PHP file could go from producing a parsers error to working fine by the simple addition of a closing tag.

Offline
Joined: 11/11/2008

I think fcool is just right with both of his suggestions.

White spaces after a closing tag can cause unwanted effects, while I never had a problem with removing them.

BTW I strongly suggest that an 1.1 version doesn't has new features but only a refomatted source code with added PHPDoc comments at least for the most important files.

alex94040's picture
Offline
Joined: 11/06/2008

Folks, this discussion is closed - it's no longer productive and is no longer taking the QCubed project forward. If you want to invest your time to help take the project forward, please do it differently: http://trac.qcu.be/projects/qcubed/wiki/WeNeedHelp.

From http://www.codinghorror.com/blog/archives/000922.html

"Thus, consensus is hardest to achieve in technical questions that are simple to understand and easy to have an opinion about, and in "soft" topics such as organization, publicity, funding, etc. People can participate in those arguments forever, because there are no qualifications necessary for doing so, no clear ways to decide (even afterward) if a decision was right or wrong, and because simply outwaiting other discussants is sometimes a successful tactic. "

Offline
Joined: 11/11/2008

Well, closing this discussion will in fact lead to nothing.

You may never had problems with trailing spaces, had you? If you have a trailing space at the wrong position it could take days to find out why your ajax action will no longer work or why the download function is broken in browser xy. Other's made that experience and decided to remove the closing tag from their project. And it IS valid PHP code as you can read in the specs.

Also, well formatted (and documented) source code seems to me essential.
And in fact I see no point why we just agree to fcool :-)

MikeHostetler's picture
Offline
Joined: 01/09/2008

Thank you everyone for sharing your opinions. As many have pointed out, there is more then one right answer.

For now, I'm going to lock this thread because I don't believe the direction of this discussion to be productive.

However, QCubed is a Community framework. At some point, we will choose a standard and we will enforce it using PHP based syntax checking tools. At that time, QCubed will facilitate the process of making the changes necessary across the entire codebase to conform with the new, community decided, coding standards.

Until that time, please help in other areas!

Thanks.