Updated PSR-1 expansion

[pmjones]

This includes changes based on comments from the earlier (now cancelled) pull request.

[ghost]

@pmjones I think github has gotten confused about this PR because it's on the master branch. You should really create a PR from a branch. Any comments made to this set of commits is getting recorded on the previous PR #15. Can you please create a branch and make that as the PR?

[winks]

Please be more explicit than "Note the placement of parentheses, spaces, and braces."
This should really be written out in detail, where exactly space/parenthes/braces should be inserted/left out.

Some of the spec has this, in 0700 and 0800 the phrase is used a few times.

[nateabele]

@Drak +1

[pmjones]

@nateabele Let me make sure I have this right. You would agree to the PSR as currently written for these items mentioned by @Drak? "The main important stuff is no white space, 4 spaces for indentation, no short php open tag, no PHP close tag, namespace and use declarations placement, defining where braces are placed, visibility on class methods and lastly camel casing on class names and methods (leaving functions out of the mix). "

[nateabele]

@pmjones Sorry, should have clarified. I was agreeing with his point about splitting them up into multiple RFCs, especially the docblock one, which definitely deserves to be its own thing (tangent: I think we could really use some innovation in the docblock department).

From @Drak's list, the things I'd agree to off the bat are: (1) no [trailing, I'm guessing?] whitespace, (2) no short open tag, (3) namespace and use declarations (though I always separate the two with an empty line, for clarity), and (4) Camel casing on class and method names.

I still think underscores make sense for private and protected methods/properties for various reasons, and leaving off PHP close tags deeply offends my very strong preference for symmetry. Call me crazy. :-) (And lets be honest, the benefits of omitting them are overstated unless you're really lazy). Finally, tabs vs. spaces belongs in a style guide, not a standard.

[pmjones]

Thanks for clarifying; I thought as much but needed to make sure. An FWIW, this is a style guide.

[ghost]

@nateabele I would say one of the most important things to get standardised is the whole tabs vs spaces thing. Overall, real tabs are pretty much frowned upon by most, people favoring x spaces. Given one of the goals here is interoperability, 4 spaces per indentation is a pretty necessary rule. This is not a matter of style, but necessity when working with a variety of libraries.

As for docblocks, personally I think they are very very necessary, but one could defer this to a separate PSR just for docblocks because there are those who bother about docblocks, and those who don't. Those who are strict can follow the PSR, and those who tend to write a few docblocks only when they feel it's necessary, can simply do their own thing. It'd be a simple thing to get ratified too because basically docblocks require a basic minimum (@params, @throws, and @return where a return value is not void). One generally doesn't require a description for class properties, just a @var typehint, and for methods a single line description of what it does is usually quite enough. I would love to add the @api convention too to public methods.

[kukulich]

Docblocks should definitely be a seperate PSR. We (me and one of my friends) already started to write it.

[nateabele]

@Drak Again, spaces vs. tabs actually has nothing to do with technical interoperability. Autoloaders see them the same. Docblock parsers see them the same. Package managers see them the same. I don't understand why we're talking about this.

For that matter, is anyone even able to give a practical, technical reason why we need this?

[pmjones]

@Drak: I have put together a "reduced" PSR-1, noting only the items you listed, and stripping out all the others. Looking at it now, I think it's not too bad. I'd rather see more, but I think you did hit the minimal useful points, without making it too minimal. Take a look at https://github.com/pmjones/fig-standards/tree/psr-1-reduced/proposed/psr-1-coding-style-guide. )I would argue that since it has been reduced so dramatically, that there should be less flexibility in deviating from it, but that's a separate topic.)

[ghost]

@pmjones Yes it does look much more doable. I wonder if you could see yourself softening the wording surrounding the 80 limit because I thought I'd configure Netbeans line limit to 80 and look over some Symfony2 code. NetBeans draws a red line down the screen at the limit. What was interesting to see is that where the code was longer than 80, it was very minimally so and splitting the lines doesnt look right in those cases.

What became clear is if there is a some complex logic, for example an if statement with a few conditions, even without taking line limits into consideration, it's just much more readable by splitting the line.

The other consideration is that, since most code is OO based these days, the actual available characters in a method is just 72 (since there are two indents, 4+4 to the actual code in the method). If you look, your vision is actually centered to where you are looking, so you an see, the 80 characters width is from the left of what you are reading, and not from the left of the screen or window. But given we count from the left of the text area, we need to take that into account.

Here are some examples that are just over the 80 limit that would not be good to split (remember these are colour coded too by the editor)

over by 5 (but actually it's just 77 long)"

$this->dispatcher->addListener('test', function ($event) use (&$dispatcher) {

this is 87 long but you'd need to split it at either the comma or the bracket, it's sort of too much.

parent::removeListener($eventName, array($this->listeners[$eventName][$key], $method));

The following is way over, at about 100. But even splitting this it would still be way over. However, it's not unreadable because the IDE colours the string.

$this->assertInstanceOf('Symfony\Tests\Component\EventDispatcher\TestEventSubscriberWithPriorities', $listeners[0][0]);

split and it's still over the 80 limit (remember these examples are all inside methods which are already indented 4+4). I this case it's actually 88 (from the left of the editor window) after being split onto separate lines.

$this->assertInstanceOf(
    'Symfony\Tests\Component\EventDispatcher\TestEventSubscriberWithPriorities', 
    $listeners[0][0]
    );

So what I'm showing is that we need to soften the wording a little. This rule will definitely become a PHPCS sniffer rule and we need to at least be able to set hard and soft limits. I'd personally set the soft limit at something like 90-100, and the hard limit at 120. PHPCS violations get marked and graphed (as warning or errors) so we need to be able to distinguish. Being able to recommend as well as flag a violation is particularly useful. The use of automated tools for styling is very much on the increase and it's the only way to really audit a large codebase anyhow. See the example at http://ci.zikula.org/job/Zikula_Core-1.3.3/violations/ where is reads (check style).

My main two points are this:

• Colouring by the editors/IDEs vastly increases readability because in terms of cognition, your brain, unlike with plain text, it already given chunks, they dont have to be separately picked out by cognisance.
• In real terms for method code, we need to count from the beginning of the indentation which is already at 8 character, this means we're immediately up to 88 from the left of the editor window.

Do you see my point?

On another note, I think we might have missed another very important think - line endings. Line endings should probably be just LF. It's generally less of a problem these days because at least with GIT, that's handled at checkout if you have the right settings, but overall, almost everyone demands that files in the repo are stored with LF line endings. That means the editors/IDEs should be configured to use LF.

[ghost]

Maybe we can do the following in https://github.com/pmjones/fig-standards/blob/psr-1-reduced/proposed/psr-1-coding-style-guide/0200-indenting-and-lines.md

add

Line Endings
----------------

Please use `LF` line endings.  This helps to avoid problems with diffs, patches and history.

And regarding the line endings use how nice does this look?

Try to limit lines to around 80-90 characters in length and where necessary, split single
statements onto separate lines. The soft limit for automated code style checker tools is
100 characters from the left margin to indicate that code should probably be edited; and
the hard limit is 120 characters per line from the left margin for generating errors. This
allows for some leeway to format code in the best readable way without heavily impinging 
on personal preferences.

It must be reiterated that where possible, you should aim for the preferred line length of
approximately 80-90 characters.

This is based on known human cognitive limitations, not the technical limitations of
screen windows or text editors and takes into account syntax highlighting present
in most text editors and IDEs.

The use of 90 here takes into account the potential lost characters from indentation of class methods (4+4). It means we can safely create PHPCS rules that will not generate huge reports and can generate warnings and errors with automated checking tools.

[bobthecow]

@Drak 👍 I can get behind 80-90 / 120.

[milesj]

Just my 2 cents:

0100-php-tags.md
0500-variable-and-property-names.md
Should be common sense for most PHP developers by now.

0200-indenting-and-line-length.md
This should not be part of the standard. This is simply preference and has nothing to do with PHP itself. We shouldn't be bringing tabs vs spaces arguments nor should we worry about max line length. So you have to scroll a few inches 1% of the time, not a big deal.

0300-namespace-use-and-class.md
No to namespaces in StudlyCase; if I want lowercase package names, it's my choice. Yes to use placement. No to placing curlies on their own line. Indifferent to multiple interfaces.

0400-constants.md
Yes, as this pertains to PHP and compatibility with other frameworks / libraries.

0600-variable-and-property-assignment.md
0800-function-and-method-calls.md
No, this is preference only, not PHP compatibility / interoperability.

0700-function-and-method-declarations.md
The order of final, static, visibility and abstract shouldn't matter.

0900-control-structures.md
0950-operators-and-expressions.md
Indifferent for the most part.

[pmjones]

Closing after subsequent edits; will open another one as needed.