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

why is space-in-parens disabled? #593

Closed
pigoz opened this issue Nov 27, 2015 · 11 comments
Closed

why is space-in-parens disabled? #593

pigoz opened this issue Nov 27, 2015 · 11 comments

Comments

@pigoz
Copy link
Contributor

pigoz commented Nov 27, 2015

Hello! This is a question not a bug :)

Looking at your examples you always use the following style: if (x) {, functionCall(a, b, c). So, I was wondering why is space-in-parens completely disabled in your eslint config?

@ljharb
Copy link
Collaborator

ljharb commented Nov 27, 2015

Where exactly would you expect to put the spaces in the parens there? Per our current style, there's spaces inside braces ({ }), but there shouldn't be any inside parens (( )) or brackets ([ ]).

@pigoz
Copy link
Contributor Author

pigoz commented Nov 27, 2015

I was thinking "space-in-parens": [2, "never"] might reflect better what is written in the styleguide.

Right now I am not getting any linting warnings when adding a space inside parens or brackets.

EDIT: looks like the following would match more closely the written syleguide (unless I am missing something obvious?).

"space-in-parens": [2, "never"],
"array-bracket-spacing": [2, "never"],
"computed-property-spacing": [2, "never"],
"object-curly-spacing": [2, "always"]

@ljharb
Copy link
Collaborator

ljharb commented Nov 27, 2015

Looks like you're right, thanks for elaborating! a PR to apply those settings is welcome.

pigoz added a commit to pigoz/javascript that referenced this issue Nov 28, 2015
pigoz added a commit to pigoz/javascript that referenced this issue Nov 28, 2015
pigoz added a commit to pigoz/javascript that referenced this issue Nov 28, 2015
@pigoz
Copy link
Contributor Author

pigoz commented Nov 28, 2015

I created the PR. Closing this issue!

@pigoz pigoz closed this as completed Nov 28, 2015
@ljharb
Copy link
Collaborator

ljharb commented Nov 28, 2015

Let's leave the issue open until the PR is merged, which will auto-close the issue :-)

@ljharb ljharb reopened this Nov 28, 2015
pigoz added a commit to pigoz/javascript that referenced this issue Nov 28, 2015
@ljharb ljharb closed this as completed in 6debbcd Nov 30, 2015
@mgcrea
Copy link

mgcrea commented Dec 3, 2015

Why use a different rule for objects? Imo "object-curly-spacing": [2, "always"] should be switched to never to follow array-bracket-spacing.

Either its {foo: 'bar'} and ['foo', 'bar'], or it's { foo: 'bar' } and [ 'foo', 'bar' ], but a mix of both is awkward & confusing.

@ljharb
Copy link
Collaborator

ljharb commented Dec 3, 2015

It's an aesthetic rule. Subjectively, it looks weird for objects to not have spaces, and it looks weird for arrays to have spaces.

At by rate, it's the rule we use inside Airbnb. You're free to fork the styleguide to apply any form of consistency you like!

@tommydangerous
Copy link

I second it looks weird for objects to not have spaces, and it looks weird for arrays to have spaces.

dustinmartin pushed a commit to dustinmartin/javascript that referenced this issue Jan 4, 2016
dustinmartin pushed a commit to dustinmartin/javascript that referenced this issue Jan 4, 2016
dustinmartin pushed a commit to dustinmartin/javascript that referenced this issue Jan 4, 2016
gilbox pushed a commit to gilbox/javascript that referenced this issue Mar 21, 2016
gilbox pushed a commit to gilbox/javascript that referenced this issue Mar 21, 2016
gilbox pushed a commit to gilbox/javascript that referenced this issue Mar 21, 2016
@jesucarr
Copy link

I think adding spaces to objects makes it less readable because it can be confused with a block (block-spacing rule), specially if it is a inline block.

Also, since arrays don't have spaces, and they are also "objects", it would be more congruent to treat them in the same way.

I understand it is your current style in Airbnb, but perhaps you might consider favour readability and congruence over the "looks weird" reasoning.

@ljharb
Copy link
Collaborator

ljharb commented May 19, 2016

"It looks weird" relates to readability, and while arrays are indeed technically objects, conceptually they should only be thought of and worked with as atomic lists. It's OK that they're treated differently.

Overall tho, the thinking is that lists don't need padding, because it's clear what separates each item - but things with key/value pairs need the padding, because it's not necessarily as clear.

@jesucarr
Copy link

Ok I can see your thinking, however objects don't necessarily have more than one item, and with the new assignment shorthand, objects don't necessarily have key/value pairs. If the object has more than a couple of items, they should go in different lines anyways. When scanning code I find more useful to clearly distinguish a block from an object, than to see what separates each item.

jaylaw81 pushed a commit to appirio-digital/ads-best-practices that referenced this issue Sep 19, 2017
jaylaw81 pushed a commit to appirio-digital/ads-best-practices that referenced this issue Sep 19, 2017
jaylaw81 pushed a commit to appirio-digital/ads-best-practices that referenced this issue Sep 19, 2017
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

No branches or pull requests

5 participants