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

brace-style 1tbs with allowSingleLine should not allow else on separate line #12284

Closed
kardasis opened this issue Sep 18, 2019 · 7 comments
Closed
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 documentation Relates to ESLint's documentation 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

@kardasis
Copy link
Contributor

What rule do you want to change?
brace-style

Does this change cause the rule to produce more or fewer warnings?
more

How will the change be implemented? (New option, new default behavior, etc.)?
new default behavior

Please provide some example code that this change will affect:

      if (condition) { doThisThing() } else {
        otherwiseDoThis()
      }

What does the rule currently do for this code?
It does not produce an error

What will the rule do after it's changed?
Produce an error indicating that you are mixing allowSingleLine with multiple lines

Are you willing to submit a pull request to implement this change?
yes

@kardasis kardasis added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules triage An ESLint team member will look at this issue soon labels Sep 18, 2019
@g-plane g-plane added 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 Sep 22, 2019
@g-plane
Copy link
Member

g-plane commented Sep 22, 2019

Thanks for this issue.

However, allowSingleLine doesn't mean enforceSingleLine. That is, you can put them in one line, but not forced.

@kardasis
Copy link
Contributor Author

It seems to me that the example I gave is a weird mix of single line and 1tbs since it’s not valid 1tbs, but it’s also not a single line statement.
If nothing else, the docs should make a mention one way or the other.

@kaicataldo
Copy link
Member

PRs to improve documentation are always welcome!

@kardasis
Copy link
Contributor Author

Cool. I’d be happy to contribute.
Would you consider an additional option that clarifies this? Something like allowSplitSingleLines

@kaicataldo
Copy link
Member

kaicataldo commented Sep 23, 2019

@kardasis If you're proposing a rule change, do you mind filling out the rule change template with the updated proposal? Either updating the original comment or adding a new comment would be fine! Once we have a formal proposal, the team can evaluate it.

@kardasis
Copy link
Contributor Author

I'll be submitting a PR with the docs shortly.
Honestly, i found myself using the docs and tests to point out some funny edge cases that I think should behave differently (or at least accept an option to behave differently) .
take a look and let me know what you think

kardasis added a commit to kardasis/eslint that referenced this issue Sep 26, 2019
add documentation and tests demonstrating iffy edge cases
kardasis added a commit to kardasis/eslint that referenced this issue Sep 26, 2019
add documentation and tests demonstrating iffy edge cases
kardasis added a commit to kardasis/eslint that referenced this issue Sep 26, 2019
Add documentation and tests demonstrating iffy edge cases
kardasis added a commit to kardasis/eslint that referenced this issue Sep 26, 2019
Add documentation and tests demonstrating iffy edge cases
@g-plane g-plane added the documentation Relates to ESLint's documentation label Sep 27, 2019
@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Oct 28, 2019
@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.

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 documentation Relates to ESLint's documentation 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