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

Comments

@kardasis
Copy link
Contributor

@kardasis kardasis commented Sep 18, 2019

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

@g-plane
Copy link
Member

@g-plane 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

@kardasis kardasis commented Sep 22, 2019

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

@kaicataldo kaicataldo commented Sep 23, 2019

PRs to improve documentation are always welcome!

@kardasis
Copy link
Contributor Author

@kardasis kardasis commented Sep 23, 2019

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

@kaicataldo
Copy link
Member

@kaicataldo 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

@kardasis kardasis commented Sep 26, 2019

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
@eslint-deprecated
Copy link
Contributor

@eslint-deprecated eslint-deprecated bot commented Oct 28, 2019

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.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants