-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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 "multiline-const", "multiline-let" and "multiline-var" STATEMENT_TYPEs to padding-line-between-statements rule #10721
Comments
I'll champion this. @eslint/eslint-team Can we get some 👍s on this please? |
@platinumazure Any news on this? :) |
@neemzy Thanks for following up. Trying to muster support again 😄 Hoping this might be accepted in a few days, otherwise we'll have to close per our issue backlog policy. |
@mysticatea @kaicataldo @not-an-aardvark Any thoughts on this proposal? |
Is the intent that this should change the behavior of the existing |
Thanks @not-an-aardvark, I didn't see the PR. I would consider the PR as breaking at this point. I think we should support the multiline-only case in a semver-minor release and change the meaning of the existing types in a semver-major. |
To me, it seems confusing to have a type called |
So |
@neemzy Please do, thanks! |
@platinumazure Should be good! I also added back the tests I had removed since they are passing again this way. |
@not-an-aardvark Given that the proposal and PR have both been tweaked to address your concerns, are you 👍 to this proposal now? @kaicataldo @ilyavolodin Any thoughts on this proposal? |
Sorry, I seemed to have missed this. I'm generally in favor of this proposal, but I do think it's a bit confusing to be consistent between Also, what is the expected behavior when there is a conflict between, say, |
Thanks @kaicataldo for your feedback!
I agree that would be confusing, but I don't think that's the case. From the current version of the rule:
So this proposal maintains consistency:
Good question. It would be nice if we could say multi or single line always wins out as it's more specific. However, we could treat these the same as how the rule treats any other conflict: "If a statement pair matches multiple configurations, the last matched configuration will be used." So we could optionally encourage users to specify the generic |
Thanks for answering my questions. One final question regarding the behavior of the last declared configuration winning - would a conflict between I think it would be probably clearest if either |
I disagree, I think that inconsistency would have the potential to create more confusion. It seems like the statement types in |
I forgot about |
I reckon it means my PR's current implementation is OK for you? :) |
What rule do you want to change?
padding-line-between-statements
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 accepted values for
prev
andnext
configuration optionsPlease provide some example code that this change will affect:
What does the rule currently do for this code? The rule is unable to differentiate single-line and multi-line
const
,let
andvar
statements.What will the rule do after it's changed? The rule will offer the possibility to treat aforementioned cases differently, e.g. enforce blank lines around a multi-line variable declaration but not around a single-line one.
The text was updated successfully, but these errors were encountered: