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

Enable rule `padding-line-between-statements` #1342

Open
furstenheim opened this issue May 20, 2018 · 5 comments

Comments

@furstenheim
Copy link

commented May 20, 2018

Hi,
I'd like to propose a new rule to standard config. When having a chain of if conditions the following is not linted as an error:

const b = 1
if (b >= 1) {
  console.log('Greater than 1')
} if (b >= 0) {
  console.log('Between 0 and 1')
} else {
  console.log('Smaller than 0')
}

This is misleading because on a quick glance the second condition would be expected to be an else if instead of an if.

Would you be interested in including a rule to avoid that possible error? I think there is no real situation where that would be something expected.

I've already written a plugin and I could write the pr to add it to the config.

Cheers

@feross

This comment has been minimized.

Copy link
Member

commented May 23, 2018

I opened an issue about this on ESLint a while ago: eslint/eslint#7116

They added a rule specifically to address this: padding-line-between-statements. We just need to enable it.

@feross feross changed the title Lint if conditions in the same line Enable rule `padding-line-between-statements` May 23, 2018

@furstenheim

This comment has been minimized.

Copy link
Author

commented May 28, 2018

I'm not completely convinced about that rule. I see the same problem that you mention in that issue. By enabling that rule you are forced to leave a line between if statements. So:

if (...) {
  // something
}
if (...) {
  // another if
}

would be disallowed. Moreover, since it can be "automatically" fixed, standard --fix would probably add an empty line between the if statements where it might have intentional to leave them together.

@LinusU

This comment has been minimized.

Copy link
Member

commented May 29, 2018

I personally think that looks better with an empty line between it ☺️

if (...) {
  // something
}

if (...) {
  // another if
}
@brodybits

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

Add this to the list for Standard 14 (#1321)?

@feross

This comment has been minimized.

Copy link
Member

commented Jul 28, 2019

It seems like the rule that ESLint added doesn't work for our purposes, but I may have misunderstood something, so I just opened an issue to clarify: eslint/eslint#12033

@feross feross transferred this issue from standard/eslint-config-standard Jul 28, 2019

@feross feross added this to the standard v15 milestone Jul 28, 2019

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