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

space between parens () and brakets []? #1303

Closed
chiefjester opened this issue Apr 16, 2017 · 29 comments
Closed

space between parens () and brakets []? #1303

chiefjester opened this issue Apr 16, 2017 · 29 comments
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:needs discussion Issues needing discussion and a decision to be made before action can be taken

Comments

@chiefjester
Copy link

Thanks for making this!

With 1.0 released; Finally we can adopt this in our company. I just wanted to open this issue in the hope it could be added as an option for future releases.

Basically adding spaces between parens and brackets.

Function arguments:
So:

function foo( one, two, three ){

Instead of

function foo(one, two, three){

Arrays:

var array = [ 1, 2, 3, 4 ]

instead of:

var array = [1, 2, 3, 4]

Methods:

array.slice( 0, 1 )

instead of

array.slice(0, 1)

This is an actual option in ESLint so I'm sure there are people using this and not just us. For me its so much easier to read a code with consistent spacing. Here's hoping you would consider this change.

@vjeux vjeux added the status:needs discussion Issues needing discussion and a decision to be made before action can be taken label Apr 16, 2017
@coodoo
Copy link

coodoo commented May 12, 2017

This is probably the only thing taking us back from switching from ESLint to prettier, so huge 👍

Wondering is there any particular reason against supporting for it?

@azz
Copy link
Member

azz commented May 13, 2017

For me its so much easier to read a code with consistent spacing.

How is one way any more consistent than the other?

@chiefjester
Copy link
Author

chiefjester commented May 13, 2017

@azz I was referring to having space between object literal where you have { foo: 1 } and having space in brackets [ 1, 2 ] actually gives you consistent spacing.

Same with space in parens: foo( bar )

A lot of authors uses space between parens and brackets to enhance readability. Just to site a famous example; you can check jQuery's source:

https://code.jquery.com/jquery-3.2.1.js

excerpt

jQuery.fn = jQuery.prototype = {

    // The current version of jQuery being used
    jquery: version,

    constructor: jQuery,

    // The default length of a jQuery object is 0
    length: 0,

    toArray: function() {
        return slice.call( this );
    },

    // Get the Nth element in the matched element set OR
    // Get the whole matched element set as a clean array
    get: function( num ) {

    // Return all the elements in a clean array
    if ( num == null ) {
        return slice.call( this );
    }

    // Return just the one element from the set
    return num < 0 ? this[ num + this.length ] : this[ num ];
    },

   ...

I do hope you consider adding this option @vjeux / @jlongster 🙏

@chiefjester
Copy link
Author

Another example:
Idiomatic.js by @rwaldron. A fairly popular Javascript Style guide that has over 12,000 stars.

Under:
2. Beautiful Syntax
A. Parens, Braces, Linebreaks

// 2.A.1.1
// Use whitespace to promote readability

if ( condition ) {
  // statements
}

while ( condition ) {
  // statements
}

for ( var i = 0; i < 100; i++ ) {
  // statements
}

@vjeux
Copy link
Contributor

vjeux commented May 22, 2017

I'm sorry but we're now past 1.0 and don't intend to introduce major formatting changes nor anymore options. You'll have to do without if you want to use prettier or you can always fork it and make it look the way you want :) Sorry!

@vjeux vjeux closed this as completed May 22, 2017
@chiefjester
Copy link
Author

@vjeux may I at least know what reason for not supporting it? Also how would become major formatting change? Just an honest question, I hope it doesn't come offensive. ✌️

@vjeux
Copy link
Contributor

vjeux commented May 22, 2017

I consider a major formatting change something that would introduce changes to most of the files of a project. In this case, it's going to impact every single file and likely a majority of lines inside of the file. So it's definitely a major change :)

One thing we're trying to do with prettier is to have the minimum number of options. We are adding some options that are big deal breakers for a large part of the community such as no-semi ( #736 ) and trying to support typescript ( #13 ).

If you look at the issues for both of those, there's a huge amount of traction around them. This one in comparison is relatively quiet. I understand that for your codebase it may be a deal breaker and I'm sorry about this, but I believe that it's going to be better for the long term of prettier to keep a minimal number of options.

@rhengles
Copy link
Contributor

@thisguychris I would accept a PR to add an option for this in prettier-miscellaneous 😀

@chiefjester
Copy link
Author

@vjeux thanks for the explanation, I'm glad you took time on explaining things. My heart is at ease now. 🙇

@Coriou
Copy link

Coriou commented Jun 9, 2017

I understand the "we're past 1.0" argument, totally makes sense. But what a crying same !

@vjeux
Copy link
Contributor

vjeux commented Jun 9, 2017

@Coriou yeah... sometimes you've got to make hard trade-offs in order for the project to be successful :(

@mcquiggd
Copy link

mcquiggd commented Jul 26, 2017

Hi all,

Maybe I am missing something, but this seems to be possible...

I am using Visual Studio Code, with the Prettier Extension (and ESLint Extension), and I can achieve this for javascript / javascriptreact by setting the VS Code settings to "prettier.eslintIntegration": true, and then setting rules in .eslintrc.json such as:

    "space-in-parens": [
        1,
        "always"
    ],

This works, even if I disable the ESLint Extension, or set eslint to be disabled in VS Code preferences. Perhaps this feature has been added to Prettier since this issue was closed..?

But I am also using TypeScript, and trying to keep my styles in sync between JavaScript and TypeScript, and I can't find a way to use ESLint settings in Prettier with TypeScript, and it seems, as mentioned above, no option to set this directly in Prettier.

Has anyone found a workaround (or alternative to Prettier) that supports something like this for TypeScript? If only Prettier settings supported specifying this behaviour, it would be possible to use it as the single style formatter... sigh :)

@dominique-mueller
Copy link

I'd also really love to see this feature! How is it that we can decide about spaces within object literal brackets but not array and function brackets? I think the very wide code style described in the original issue description is very common ...

@lydell
Copy link
Member

lydell commented Dec 3, 2017

@dominique-mueller The bracket-spacing option exists because of legacy reasons. If it didn’t exist and somebody proposed it today I doubt it would have been added.

@dominique-mueller
Copy link

Hm, so it's definite that a wide code style besides the current condensed one won't happen?

@gziolo
Copy link

gziolo commented Dec 22, 2017

This feature is a blocker for the WordPress community to have it widely adopted. There are over 50,000 plugins available for WordPress and many of them contain JavaScript code that follows WordPress coding standards. The spacing rules were created based on jQuery style guide which can be summarized as follows:

Use spaces liberally throughout your code. “When in doubt, space it out.”

One of the alternatives would be to use Prettier fork created at @Automattic to use with Calypso. It isn't the solution that should be maintained for a long time. It would be great if we could opt-in for such option inside Prettier instead and take advantage of the supported Eslint integration.

@lydell
Copy link
Member

lydell commented Dec 23, 2017

@gziolo As you probably know, the --bracket-spacing option todes that, but only for { and }. However, we are investigating the possibility of removing that option: #3520. Just as a heads up. Also note that if you're not willing to let any of your current style rules go you'll have a really hard time adopting Prettier.

@mcquiggd
Copy link

mcquiggd commented Dec 24, 2017

I have to admit that I find this statement a bit ... surprising...

if you're not willing to let any of your current style rules go you'll have a really hard time adopting Prettier.

Uhm... surely, the whole point of Prettier, is to be able to enforce chosen coding style rules, while linters enforce chosen coding practices...?

Please, correct me if I have misunderstood this Projects goals - I know it openly describes itself as an 'Opinionated Code Formatter'.

So far I have found Prettier to be very useful, but the lack of options is problematic, when consulting with multiple teams that have their own style rules. Perhaps you would have a larger audience, if there was a little more flexibility, rather than so many people hacking ways to make it less opinionated?

@dominique-mueller
Copy link

I personally think that allowing devs to choose between a condensed and a wide code style is one of the options that should exist. So many projects (including the ones I'm working on) prefer the wide code style with more spacing.

In my opinion, there's no difference of this option vs. let's say semicolons or tabs / spaces - that's why I also don't understand why it does not exist yet. I think preferring a wide code style is a valid, very popular opinion ...

@gziolo
Copy link

gziolo commented Dec 30, 2017

@gziolo As you probably know, the --bracket-spacing option todes that, but only for { and }. However, we are investigating the possibility of removing that option: #3520. Just as a heads up.

{ and } is a very limited subset of what I referred to. I agree that this rule is too granular and I understand why you would want to deprecate it. Thanks for letting me know.

Also note that if you're not willing to let any of your current style rules go you'll have a really hard time adopting Prettier.

@lydell I'm totally aware of it, but as I already mentioned above spaces around most of the operators are the only concern. You can compare the difference between Prettier and calypso-prettier fork to better understand what is missing here: master...Automattic:calypso-1.9. @jsnajdr can give more details about the implementation if necessary.

@ahmadawais
Copy link

Just wanted to restate what @gziolo said. WordPress is a huge eco-system. About 30% of the internet sites are powered by WordPress and more than 70,000 known WordPress themes and plugins. That's 30,000+ developers who follow the spacing guideline set by WordPress coding standards based on the jQuery's stance.

Adopting this one configuration would go a long way in adoption of Prettier in the huge WordPress eco-system which can easily be compared to React/Angular ecosystem as it's bigger in size and diversity.

I say this as a core contributor and as a blogger trying to build an open source boilerplate for the WordPress community that would just work with ESLint and Prettier.

Peace! ✌️

@paulocoghi
Copy link

paulocoghi commented Jan 2, 2018

The lack of spaces between parentheses, brackets and braces is the only reason I do not switch from ESLint to Prettier.

@adrienharnay
Copy link
Contributor

Not sure if it has been said before, but just use prettier-eslint. Prettier will run first, and eslint second (with —fix). Don’t forget the prettier config for eslint to disable rules in conflict with prettier though, and manually activate what you want to enforce!

@paulocoghi
Copy link

@Zephir77167 , thanks for the tip! But it is so simpler to just use ESLint, which I see no sense in using prettier-eslint.

@duailibe
Copy link
Member

@paulocoghi because ESLint and Prettier are tools with different purposes :) ESLint won't format your code like Prettier, Prettier can't be configured as much like ESLint. Some people have found a sweet stop with prettier-eslint, which lets prettier do the heavy formatting and then just fix the nits you want with eslint --fix.

@chiefjester
Copy link
Author

chiefjester commented May 31, 2018

@vjeux I do hope you revisit this issue, you said there was no traction before hence you close it. But I believe this is not the case now, a lot of people have voiced out probably because prettier is more known now thanks to its adoption.

The JS style guide that I mentioned, pretty much predates a lot of the developers' career here. Where Idiomatic.js first commit was around May 2011 and jQuery style guide even older. If you are looking for golden js style guide, this is probably one of the oldest and is very much relevant and widely adopted.

Also now as @ahmadawais stated, WordPress style guide uses that too? That's HUGE right? If you are looking for traction then this is it.

Here's hoping you reconsider.

@lucasestevao
Copy link

I hope this new feature to be released as soon as possible.

@chinchang
Copy link
Contributor

I don't think we need a separate option for this. bracketSpacing sounds like it should be doing just this.

@paulocoghi
Copy link

@chinchang , the last time I tested it was not adding spaces between parentheses, brackets and braces.

@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Dec 13, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Dec 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:needs discussion Issues needing discussion and a decision to be made before action can be taken
Projects
None yet
Development

No branches or pull requests