JS Style Guide: Remove spacing exceptions

[mikesherov]

Motivation:

1. A simple style guide that makes it easy for contributors to contribute and easy for style guide enforcement software to enforce.
2. Arguments about "what looks good" are subjective. What should dictate style is principal of least surprise and least exceptions to the rule, except where the exceptions are objectively better or necessary.

Yes, this would require some changes to current code. @jzaefferer and I can handle that easily with esformatter and jscs should this be approved.

Thoughts? /cc @dmethvin @scottgonzalez @timmywil @gibson042 @markelog

[mgol]

Yeah, it's getting confusing even for me sometimes so what are poor out-of-the-team contributions to do?

[timmywil]

We just keep adding more and more spaces to type. At some point, it's going to look like there's a space between every character; it seems the only reason here is that it's easier to enforce. I'm somewhat indifferent, but I like the the way the current code looks.

[mikesherov]

@timmywil only more spaces on certain edge cases. Easy to enforce and easier to remember less exceptions.

If you're indifferent, can you qualify that by saying that you wouldn't be opposed to this PR despite your preference for the current way it looks?

[timmywil]

Easy to enforce and easier to remember less exceptions.

Is it really that hard to remember?

can you qualify that by saying that you wouldn't be opposed to this PR despite your preference for the current way it looks?

That's what I meant, but I wanted to voice my preference in case others agree.

[gibson042]

I like the "collapse whitespace around single items when those items are already bounded by punctuation" exceptions not because they look good, but because they're easier to read, which is admittedly still subjective.

That said, I'll accept whatever's in the style guide, even if it is motivated by machine enforcement. So pretty much "what @timmywil said".

[mikesherov]

Is it really that hard to remember?

Given the number of comments on spacing... yes, apparently. Especially when you're modifying. e.g.:

• }); -> }, whatever ); you need to remember adding that space, and in the case of:
• call({ a: b }); -> call( a, { a: b } ); you need to remember the extra space at the end.

[scottgonzalez]

This isn't just about machine enforcement. Our style guide has always been a huge PITA. It's largely subjective and just causes lots of "Please fix X" comments in PRs for no objective benefit. One of the big goals here is to make the style guide as objectively good as possible and eliminate exceptions that are purely subjective because what they do is make the style guide longer and harder to understand, therefore increasing the potential for violations without increasing the objective quality of the code.

[scottgonzalez]

I feel like we can actually drop the Array and Function Call Spacing section completely since they no longer contain exceptions. There's certainly a lot of overlap with the Object and Array Expressions section. We can probably just collapse it all to Bad Examples and Good Examples.

[arschmitz]

+1 for consistency here. Even though I like certain things about how our current guide looks, it's a HUGE PITA when doing pr reviews. Even I still am not always clear on every case. This makes it much much easier to understand.

[dmethvin]

Aren't all style guides subjective? What makes this more objective?

The main drawback I see with our "spaces everywhere" policy is that it's hard to find anyone else putting that many spaces into their source code. It's also not a typical style most other people use for JavaScript outside jQuery projects. In particular, spacing around array brackets and function call args is pretty rare elsewhere.

[scottgonzalez]

Aren't all style guides subjective?

Absolutely not. Most style guides are highly subjective, but that doesn't mean that all are or even that you need to include a single subjective rule in a style guide.

What makes this more objective?

This change is not about making it more objective. This change is about avoiding complexity via subjective exceptions.

An objective change would be removing onevar.

[mikesherov]

This change is not about making it more objective. This change is about avoiding complexity via subjective exceptions.

This.

The main drawback I see with our "spaces everywhere" policy is that it's hard to find anyone else putting that many spaces into their source code.

And yet, you find no one who has the "spaces everywhere except nested first or last etc." rule. Most are either spacey, or not spacey. Those who are spacey are more consistent then us.

In particular, spacing around array brackets and function call args is pretty rare elsewhere.

yes. Most projects use: [1, 2, 3] vs. [ 1, 2, 3 ], but that is a red herring for this discussion. This isn't about making our style guide more spacey, it's about making it less complex.

[mikesherov]

In particular, spacing around array brackets and function call args is pretty rare elsewhere.

@dmethvin , also if you want to propose no spaces in array expressions or accessors, I'm fine with that. The two issues are unrelated. This is solely about the "first, last multiline exception".

[markelog]

My two cents here, i think current exceptions are complicated. I think it would be cool to make them less comlicated. But not remove them complitly.

I think exceptions might stay for the array/object literals and function token, but regardless of multiline/singleline business and amount of arguments:

foo([ a, b ]);
foo([ 
    a, b 
]);

// Less pretty, but consistent
foo( a, [ a, b ]);

foo({ a: "alpha", b: "beta" })
foo({ 
    a: "alpha", b: "beta" 
})

// Less pretty, but consistent
foo( a, { a: "alpha", b: "beta" })

foo(function() {
    ...
}, options );

foo(function() {
    ...
})

foo(function() { ... })

// Less pretty, but consistent
foo(function() { .. }, options )

( a + b )
// Consistent!
( true ? 1 : { a: b })

Same would apply not only for call expressions but for the array/object literals. In fact, i would extract relevant entries from "Object and Array Expressions" and put them under "Spacing" section and throw way everything else.

I think it would be consistent and intuitive that way, easy to enforce too. And yeah, current exceptions for parentheses are horrible, have nightmares about their implementation, especially those singleline/multiline and arguments length requirements, main reason why jscs is not full supports jquery code-style yet. So if would decide to go with these changes or with the way above today, jscs could support it tomorrow.

In particular, spacing around array brackets and function call args is pretty rare elsewhere.

Actually, https://github.com/rwaldron/idiomatic.js uses it, which is fairly popular code-style and all wikimedia projects, so we're not alone, not alone at all :-).

[mikesherov]

I'd be fine with @markelog's suggestion too, but really I think the less exceptions the better. Perhaps we can take a vote?

1. as is.
2. as I've suggested.
3. as @markelog has suggested.

Please don't vote 1. For the sake of everyone's sanity who's trying to automate things.

[scottgonzalez]

I vote for 2.

[arschmitz]

2

[timmywil]

3

[sfrisk]

I vote 2 as well.

[mgol]

2 from me.

[dmethvin]

Bottom line, 2.

I agree that consistency would help. If it were my own coding preference I'd like to get there by taking spaces out in several places, but given the choices I agree that 2 is more consistent.

[markelog]

Cool, sounds like we are good to go.

[mikesherov]

I'm going to merge this tomorrow AM. Thanks for the concessions @timmywil and @gibson042 !

[mikesherov]

Err, that is, I'll merge it tomorrow with some more consolidation of examples. Thanks again everyone!

[jzaefferer]

@mikesherov anything holding this up?

[mikesherov]

@jzaefferer nope, just juggling some stuff. Will be done shortly.

[mikesherov]

Thanks again everyone for the input. This has been pushed and tagged as 1.1.3