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

Add third, no-op option accepted in places where only "always" and "never" are accepted #10906

Closed
pineapplemachine opened this issue Oct 1, 2018 · 5 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules

Comments

@pineapplemachine
Copy link
Contributor

I believe that there would be value in adding a third, "off" or similar option to places that currently accept only "always" and "never". Where "always" requires that a condition always be met, and "never" requires that it never be met, "off" (or a similar setting) would ignore that condition and not enforce the condition either way.

The space-before-blocks rule is what prompts me to post this. I and another dev have been discussing code style and enforcing this with eslint, and while we both agree that there should be spaces before function and class blocks we feel less decisive regarding spaces between other blocks, such as if and for code blocks. At least for the time being, we want to use the formatter to enforce space before function and class blocks only. We do not want eslint to produce warnings for either style (space or no-space) for other blocks.

I expected to be able to write this:

"space-before-blocks": ["warn", {
    "functions": "always",
    "classes": "always",
    "keywords": "off",
}],

Which did not work as I hoped.

I also tried omitting the "keywords" property, but in this case eslint behaves as though I wrote "always".

This is an example of the behavior I would expect given the aforementioned space-before-blocks rule.

// OK
function myFunction() {
    ...
}
// NOT OK
function myFunction(){
    ...
}

// OK
class MyClass {
    ...
}
// NOT OK
class MyClass{
    ...
}

// OK
if(x) {
    ...
}
// OK
if(x){
    ...
}
// OK
for(x of y) {
    ...
}
// OK
for(x of y){
    ...
}
@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Oct 1, 2018
pineapplemachine added a commit to pineapplemachine/eslint that referenced this issue Oct 1, 2018
See eslint#10906

Where "always" enforces space and "never" enforces no-space, "off" does not enforce any space preference.

It may be useful to add a similar "off" option to other enums only allowing "always" and "never", too, but that is outside the scope of this PR.

TODO: Why do two tests fail? (what the heck???)
TODO: The documentation will need to describe this addition in more detail.
pineapplemachine added a commit to pineapplemachine/eslint that referenced this issue Oct 1, 2018
See eslint#10906

Where "always" enforces space and "never" enforces no-space, "off" does not enforce any space preference.

It may be useful to add a similar "off" option to other enums only allowing "always" and "never", too, but that is outside the scope of this PR.

TODO: Why do two tests fail? (what the heck???)
TODO: The documentation will need to describe this addition in more detail.
pineapplemachine added a commit to pineapplemachine/eslint that referenced this issue Oct 1, 2018
See eslint#10906

Where "always" enforces space and "never" enforces no-space, "off" does not enforce any space preference.

It may be useful to add a similar "off" option to other enums only allowing "always" and "never", too, but that is outside the scope of this PR.

TODO: Why do two tests fail? (what the heck???)
TODO: The documentation will need to describe this addition in more detail.
@nzakas
Copy link
Member

nzakas commented Oct 1, 2018

Thanks for the issue. It sounds like you are proposing a change to space-before-blocks. Can you please update this issue using the correct template for this request?

@pineapplemachine
Copy link
Contributor Author

@nzakas I did not feel that the template was a good fit because I am proposing a change that might affect any rules which currently accept only "always" and "never" values, though the rule where not having any third no-op option is affecting me right now is in fact space-before-blocks. Unfortunately, I do not have a thorough enough knowledge of eslint and its rules to enumerate all of the rules that would benefit from such a change.

@nzakas
Copy link
Member

nzakas commented Oct 3, 2018 via email

@pineapplemachine
Copy link
Contributor Author

Ok, I understand. Note that I do currently have a PR open for adding this functionality to space-before-blocks specifically, here: #10907

@kaicataldo kaicataldo added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Oct 15, 2018
nzakas pushed a commit that referenced this issue Nov 9, 2018
* Update: "off" options for "space-before-blocks" (refs #10906)

See #10906

Where "always" enforces space and "never" enforces no-space, "off" does not enforce any space preference.

It may be useful to add a similar "off" option to other enums only allowing "always" and "never", too, but that is outside the scope of this PR.
@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Dec 11, 2018
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jun 10, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jun 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

3 participants