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

Curly: multi, but with balanced if-else statements #2390

Closed
bathos opened this issue Apr 27, 2015 · 5 comments
Closed

Curly: multi, but with balanced if-else statements #2390

bathos opened this issue Apr 27, 2015 · 5 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules

Comments

@bathos
Copy link

bathos commented Apr 27, 2015

I came here because I ran into the problem at #2366, and it's awesome to see it was just fixed yesterday! I had two other concerns related to brace style as well, both of which are feature requests rather than bug reports.

The first I discovered was already detailed at #1806.

The other I didn't find described yet. I'm curious whether a style option extending 'multi' for curly might be considered, for handling if/else blocks nicely. In my experience those of us who favor multi will nonetheless wish to balance if/else blocks when one or the other is multi-statement. That is, if either 'if' or 'else' is a multi-statement block, the other should use braces as well, or else you end up with rather awkward results.

So the legal forms would be, as with multi:

if (condition)
    statement();
else
    statement();

and of course:

if (condition) {
    statement();
    statement();
}  else {
    statement();
    statement();
}

but also, this, which is considered an error when using 'multi':

if (condition) {
    statement();
    statement();
} else {
    statement();
}

instead of the form which is currently considered valid (and which I've rarely seen used in practice, for reasons that seem apparent):

if (condition) {
    statement();
    statement();
} else
    statement();
@nzakas nzakas added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion labels Apr 27, 2015
@nzakas
Copy link
Member

nzakas commented Apr 27, 2015

We're getting a bit into fuzzy areas here. I think what you're asking for is that if one branch of the if statement has multiple statements then all branches must use curlies, is that correct?

I'd call this "multi a consistent".

@bathos
Copy link
Author

bathos commented Apr 27, 2015

Yes, that's correct.

@BenoitZugmeyer
Copy link
Contributor

Interesting option, I'd like to use it. Working on it.

BenoitZugmeyer added a commit to BenoitZugmeyer/eslint that referenced this issue Oct 3, 2015
BenoitZugmeyer added a commit to BenoitZugmeyer/eslint that referenced this issue Oct 3, 2015
BenoitZugmeyer added a commit to BenoitZugmeyer/eslint that referenced this issue Oct 3, 2015
@BYK
Copy link
Member

BYK commented Oct 4, 2015

I was gonna propose this actually. We need this at work too :) 👍

BenoitZugmeyer added a commit to BenoitZugmeyer/eslint that referenced this issue Oct 5, 2015
BenoitZugmeyer added a commit to BenoitZugmeyer/eslint that referenced this issue Oct 5, 2015
BenoitZugmeyer added a commit to BenoitZugmeyer/eslint that referenced this issue Oct 5, 2015
@nzakas nzakas closed this as completed in 5a24e79 Oct 6, 2015
nzakas added a commit that referenced this issue Oct 6, 2015
New: add "consistent" option to the "curly" rule (refs #2390)
@aparajita
Copy link
Contributor

instead of the form which is currently considered valid (and which I've rarely seen used in practice, for reasons that seem apparent)

FYI: With non-allman style braces, I agree. However, with allman style braces, inconsistent use of braces looks quite natural (to me and my colleagues at least):

if (foo)
{
    doSomething();
    doSomethingElse();
}
else
    doBar();

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 2018
@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 Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

5 participants