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

Consider to add `class` as an additional configuration in `space-before-blocks` rule #4089

Closed
almirfilho opened this Issue Oct 8, 2015 · 8 comments

Comments

Projects
None yet
5 participants
@almirfilho
Copy link

commented Oct 8, 2015

What do you guys think about adding an additional config in space-before-blocks rule for ES6 classes?
Actually there are only two possible options: functions and keywords. However, it might be desirable to consider classes as a separated config as well. For instance, I might want my code to follow the rule as {"keywords": false, "functions": false, "classes": true}:

class Foo extends Bar {
  someMethod(){
    if(something){
      // do something
    }
  }
}

Thank you!

@eslintbot

This comment has been minimized.

Copy link

commented Oct 8, 2015

Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage label Oct 8, 2015

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Oct 8, 2015

Which version of eslint are you using?
This has already been added: #3066

@gyandeeps gyandeeps added question and removed triage labels Oct 8, 2015

@almirfilho

This comment has been minimized.

Copy link
Author

commented Oct 8, 2015

Hi @gyandeeps. I think #3066 just considers class statements to be linted with the rule space-before-blocks, but there's no way to configure spaces before class blocks separately. As we can see in the docs, there're only options for functions and keywords, but not for classes.

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Oct 8, 2015

Ah, I see. This was missed during the enhancement work done here: #3811
@nzakas @ilyavolodin Do we flag this as a bug or enhancement? I am guessing its a bug. If not feel free to change the labels.

@gyandeeps gyandeeps added rule accepted bug and removed question labels Oct 8, 2015

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Oct 8, 2015

@almirfilho Would you be willing to work on this fix?

@ilyavolodin ilyavolodin added enhancement and removed bug labels Oct 9, 2015

@ilyavolodin

This comment has been minimized.

Copy link
Member

commented Oct 9, 2015

This is an enhancement as #3811 was fixing a bug where classes were not linted at all using this rule. This request is to add new configuration option.
I'm not using es6, so my opinion is not really matter that much, but I would consider classes to be falling under function flag. Not sure I see a huge need to add another option specifically for them.

@nzakas nzakas added evaluating and removed accepted labels Oct 9, 2015

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Dec 16, 2015

@nzakas @ilyavolodin What should be done here?

@nzakas

This comment has been minimized.

Copy link
Member

commented Dec 16, 2015

Let's add the option. It's nice and explicit.

@nzakas nzakas added accepted and removed evaluating labels Dec 16, 2015

alberto added a commit that referenced this issue Dec 20, 2015

@alberto alberto closed this in c252f12 Dec 21, 2015

gyandeeps added a commit that referenced this issue Dec 21, 2015

Merge pull request #4751 from eslint/issue4089
Update: configuration for classes in space-before-blocks (fixes #4089)

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.