palantir / tslint

:vertical_traffic_light: An extensible linter for the TypeScript language
http://palantir.github.io/tslint/
Apache License 2.0
5.9k stars 887 forks source link

More lenient "indent": [true, "tabs"] #1306

Closed JoshuaKGoldberg closed 8 years ago

JoshuaKGoldberg commented 8 years ago

Consider the following formatting where ~~~> is a tab and - is a space:

~~~>if-(longArgumentlongArgumentlongArgument
~~~>---&& anotherLongArgumentanotherLongArgument) {
~~~>~~~>// ...
~~~>}

This gets flagged by the indent rule.

Would you be up for me adding an option to allow <4 spaces at the end of the tabs?

adidahiya commented 8 years ago

ooh this seems like holy war fodder. Mixing tabs and spaces? It should probably be an additional option, and you can't hard code the max space count to 4 because other users might set tabwidth to 2 or 8 (that's the whole point of tabs I believe).

JoshuaKGoldberg commented 8 years ago

Yes, holy war fodder indeed. I should have been more generic; a better proposal would be to add to the indent rule:

It could look like:

{
    "indent": [
        true,
        "tabs",
        {
            "allow-spaces": true/2/4/"infinite" // false by default, any number, or "infinite"
        }
    ]
}
olegstepura commented 8 years ago

Let me leave my 5 coins here: when we decided to use tabs for spacing in our company, we figured out that it's only worth to use tabs for all type of spacing. Like this:

if (
~~~>longArgumentlongArgumentlongArgument
~~~>&& anotherLongArgumentanotherLongArgument
) {
~~~>hereICallSomeMethod(
~~~>~~~>withLongParameterName1,
~~~>~~~>withLongParameterName2
~~~>)
}

After 5 years of using this rule (now we switched to 2 spaces) I can tell you this was the right decision!

rlasjunies commented 8 years ago

@olegstepura can you precise which one is the right decision. It's not clear for me in your comment.

Tabs or 2 spaces.

Thank

olegstepura commented 8 years ago

We switched to 2 spaces to be consistent with the rest of the world. The right decision is to use the standard which is used by most of open source in the scope of the language you operate.

We used tabs for PHP and javascript, but the rest of the world used 4 spaces for PHP and tabs (?) for javascript (jquery uses tabs). Now we use scala and most open source projects in scala use 2 spaces. So we switched to using 2 spaces for scala. Later I started using 2 spaces for Typescript as well.

It's actually up to you which one to choose, but I would recommend inspecting what is used by the open source software you will use (framework, maybe some big libraries). For PHP there is http://www.php-fig.org/psr/psr-2/ with requirement for using 4 spaces, I just googled and found that Typescipt itself uses 4 spaces, as contributed code require to use, see https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines.

But I will continue to use 2 spaces ;)

rlasjunies commented 8 years ago

Thank, I better understand your comment. And I see you point of view.

2016-06-15 15:33 GMT+02:00 Oleg Stepura notifications@github.com:

We switched to 2 spaces to be consistent with the rest of the world. The right decision is to use the standard which is used by most of open source in the scope of the language you operate.

We used tabs for PHP and javascript, but the rest of the world used 4 spaces for PHP and tabs (?) for javascript (jquery uses tabs). Now we use scala and most open source projects in scala use 2 spaces. So we switched to using 2 spaces for scala. Later I started using 2 spaces for Typescript as well.

It's actually up to you which one to choose, but I would recommend inspecting what is used by the open source software you will use (framework, maybe some big libraries). For PHP there is http://www.php-fig.org/psr/psr-2/ with requirement for using 4 spaces, I just googled and found that Typescipt itself uses 4 spaces, as contributed code require to use, see https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines.

But I will continue to use 2 spaces ;)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/palantir/tslint/issues/1306#issuecomment-226188159, or mute the thread https://github.com/notifications/unsubscribe/ABX4L2--bv7jg5NhzqhlxUqGUGnC5D_mks5qL_87gaJpZM4IzWI2 .

Richard Lasjunies

jkillian commented 8 years ago

Using spaces after tabs seems very non-standard to me. I don't care at all whether projects choose to use spaces or tabs, but I'm not convinced we want to promote any combination of both.

If we do, off-by-default as #1317 does is definitely the right way to go, but I feel hesitant that this should be part of the core rules at all.

JoshuaKGoldberg commented 8 years ago

In defense of the rule, the ideology of maintaining a single style is good to have, but it doesn't hold up in all corner cases. Take the longArgument example from the OP: some tab-using teams are purists; some prefer to line up the start of the second line with the parenthesis; some prefer with the content of the parenthesis.

~~~>if-(longArgumentlongArgumentlongArgument
~~~>~~~>&& anotherLongArgumentanotherLongArgument) {
~~~>if-(longArgumentlongArgumentlongArgument
~~~>---&& anotherLongArgumentanotherLongArgument) {
~~~>if-(longArgumentlongArgumentlongArgument
~~~>----&& anotherLongArgumentanotherLongArgument) {

Allowing spaces after the tabs doesn't violate the tab ideology of allowing any width for them because they're aligning to the characters above. It's necessary to use spaces for vertical alignment: they're synced to the text content rather than indentation level.

~~~>while-(longArgumentlongArgumentlongArgument
~~~>-------&& anotherLongArgumentanotherLongArgument) {

All that being said, if you'd prefer to keep out of this warzone that's reasonable as well.

I'm playing devil's advocate here: IMO spaces are obviously better than tabs!

olegstepura commented 8 years ago

@JoshuaKGoldberg such code becomes broken too often. Extra space can be left after tab, some editors would convert other 4 spaces to tabs and viola, we got this:

~~~>-~~>--~>some code
//[tab][space][tab][space][space][tab]

visually some code is aligned with 3 tabs for all who setup tab as 4 spaces. But those who setup tab as 2 spaces will see it badly formatted.

JoshuaKGoldberg commented 8 years ago

@olegstepura Is that sample not a violation of the proposed cases? The feature proposal is to allow a limited number of spaces after tabs, not interspersed within.

I agree with @JKillian that we shouldn't be advocating one style over others. From my understanding the debate is whether the style is legitimate enough to justify being included in tslint core at the risk of allowing (advocating for?) improper behavior.

olegstepura commented 8 years ago

@JoshuaKGoldberg you're right, my example was about mixing tabs with spaces, not just ending tabs with spaces. But that was an example of evil mixing tabs with spaces. Just choose one method and you will find a way to workaround your issue (as I mentioned earlier, for example)

jkillian commented 8 years ago

I understand why this rule would help in @JoshuaKGoldberg's situation, but overall mixing tabs and spaces doesn't seem like an idea we want to encourage, so I'm going to close this. Thanks for all the input everyone!