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

vue/padding-line-between-tags with new 'consistent' option #1978

Closed
mesqueeb opened this issue Sep 17, 2022 · 9 comments · Fixed by #1982
Closed

vue/padding-line-between-tags with new 'consistent' option #1978

mesqueeb opened this issue Sep 17, 2022 · 9 comments · Fixed by #1982

Comments

@mesqueeb
Copy link
Contributor

What rule do you want to change?
vue/padding-line-between-tags
https://eslint.vuejs.org/rules/padding-line-between-tags.html#vue-padding-line-between-tags

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

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

I don't want blank lines in between all siblings or after specific tags.

What I would love:

  • no blank lines between siblings: GOOD
  • blank lines between siblings: also GOOD
  • under the same parent blank lines between some siblings, but not others: BAD

Inconsistent spacing between same parent siblings is really inconsistent and drives me crazy. However, I do not want to force or disallow blank lines, I feel it's often a case-by-case choice depending how cluttered a parent with siblings looks.

My rule wish is:

{
  "vue/padding-line-between-tags": ["error", [
    { "blankLine": "consistent", "prev": "*", "next": "*" }
  ]]
}

Please provide some example code that this change will affect:

GOOD

<template>
  <header>
    <div></div>
    <div></div>
    <div></div>
  </header>

  <div></div>
  
  <div />
  
  <footer></footer>
</template>

GOOD

<template>
  <header>
    <div></div>

    <div></div>
    
    <div></div>
  </header>
  <div></div>
  <div />
  <footer></footer>
</template>

BAD

<template>
  <header>
    <div></div>

    <div></div>
    <div></div>
  </header>
  <div></div>

  <div />
  <footer></footer>
</template>
@FloEdelmann
Copy link
Member

CC @dev1437 and @amiranagram, follow-up to #1832 and #1966.

@dev1437
Copy link
Contributor

dev1437 commented Sep 19, 2022

#1982 @FloEdelmann, but we will definitely need more edge cases defined and tests for them. If @mesqueeb wants to try the current behaviour and see if there are any issues, that would be great.

@FloEdelmann FloEdelmann linked a pull request Sep 19, 2022 that will close this issue
@mesqueeb
Copy link
Contributor Author

@dev1437 sure! And thanks so much for this implementation!

I think it's easiest for me to test it if you could release it to npm once. @FloEdelmann are you comfortable releasing it to npm once?

I can open new issues for any lint bugs for this rule i come across in the future.

@FloEdelmann
Copy link
Member

@mesqueeb We can't just merge and release a non-tested feature. If you can't test it in a repo, maybe you can have a look at test cases in the PR #1982 and see if you disagree with the behavior there, or suggest new test cases that are likely to fail.

@mesqueeb
Copy link
Contributor Author

@FloEdelmann i'd love to run it on my codebase once. From there if I find failures, I'll add those as test cases.

I'll try to set up a fork and let you know how it goes. : )

@ota-meshi
Copy link
Member

I think we need to look more into how the new option work.
#1982 (comment)

@ota-meshi
Copy link
Member

ota-meshi commented Sep 20, 2022

I think that it is better to regard one element as one group and check whether it is consistent in the group. However, the definition of groups is currently inflexible, so it might be a good idea to change the prev and next options to allow CSS selectors.

@ota-meshi
Copy link
Member

I reconsidered that it might be better to check between elements where the matched blankLine returns consistent, instead of groups.

@dev1437
Copy link
Contributor

dev1437 commented Sep 23, 2022

I've updated this, so now it should only consider blocks that have consistent rule and are siblings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants