Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed everything to be psr2 compatible (php-cs-fixer) #2743

Closed
wants to merge 1 commit into from

Conversation

digitalkaoz
Copy link

autofixed everything to be psr2 compatible, the defacto php formatting standard nowadays. see php-fig for more details

@samdark samdark closed this Mar 13, 2014
@samdark
Copy link
Member

samdark commented Mar 13, 2014

We aren't going to use spaces.

@digitalkaoz
Copy link
Author

So why is yii trying to be different to almost every other PHP library?

@samdark
Copy link
Member

samdark commented Mar 13, 2014

See #9

@samdark
Copy link
Member

samdark commented Mar 13, 2014

It is not a final decision but we saw do drawbacks using tabs so far so see no reason to switch.

@digitalkaoz
Copy link
Author

MH the linked issue doesnt show any drawbacks, what drawbacks should a code formatting have? The current code style is almost unreadable in every IDE unless you reconfigure it, simply watch it in github, 8chars for one tab seems to be the default display mode, i really think yii should adopt common sense. If you are using 3rd Party libs in yii, you have to constantly switch coding styles, its a huge timewaster to always have to worry about what file you are editing and what code style to use. Thats why almost every phplib jumped onto the psr2 Train, except yii ;)

@nineinchnick
Copy link
Contributor

@samdark if there are no cons/pros is it just a matter of core team preference? Shouldn't this be put up for a vote? It seems more people are asking for PSR-2 compliance on this than defending tabs. I've been using tabs but I vote for spaces to stop this from popping up. I really don't care much, I want consistency between all devs.

@cebe
Copy link
Member

cebe commented Mar 14, 2014

As far as I have seen most commonly set tab width is 4 spaces, at least in modern IDE like PHPStorm. Also 8 spaces do not make it unreadable it is just a bit more indented.

A significant drawback of switching is that git blame will not work anymore for changes before the switch.

Btw: tabs allow one key press regardless of editor.
If you do a quick fix with a bad editor that does not do auto indentation you always have to type four spaces.
Can not see good reasons to switch.

@digitalkaoz
Copy link
Author

Interoperability (even in code styles) is the major pro which outweights all artificial cons...

Im using tab all day, but its converted to a Format which looks the same in every editor/OS/whatever...

Even navigation is handled properly.

Git diff can ignore whitespace changes, so even not an issue...

I would vote for a vote and that people stand back their personal preferences for a Mord consistent PHP world :)

@nineinchnick
Copy link
Contributor

@cebe moving around with spaces just requires holding ctrl and I do that anyway when using tabs to skip words instead single chars. As for editing, autoindent on a single line should be used.
Vim handles all that too well so we don't need to bring it up.

What I'm saying is that there are no STRONG pros/cons. All that is just preference, not real issues. Even using a bad editor is a preference. Who's for a vote?

PS. I indent my array values and it's more of a problem in diffs when using tabs.

@digitalkaoz
Copy link
Author

would love to see it decided as a democratic process :)

@samdark samdark mentioned this pull request Mar 14, 2014
@samdark
Copy link
Member

samdark commented Mar 14, 2014

Created separate issue for discussion and probably voting.

@rawtaz
Copy link
Contributor

rawtaz commented Apr 20, 2014

@digitalkaoz Thanks for being a jerk and polluting yet another framework with your stupid ideas of what flexible and dynamic means. Great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants