eslint-plugin-vue icon indicating copy to clipboard operation
eslint-plugin-vue copied to clipboard

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

Open mesqueeb opened this issue 2 years ago • 8 comments

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>

mesqueeb avatar Sep 17 '22 03:09 mesqueeb

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

FloEdelmann avatar Sep 17 '22 09:09 FloEdelmann

https://github.com/vuejs/eslint-plugin-vue/pull/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.

dev1437 avatar Sep 19 '22 12:09 dev1437

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

mesqueeb avatar Sep 19 '22 13:09 mesqueeb

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

FloEdelmann avatar Sep 19 '22 18:09 FloEdelmann

@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. : )

mesqueeb avatar Sep 19 '22 23:09 mesqueeb

I think we need to look more into how the new option work. https://github.com/vuejs/eslint-plugin-vue/pull/1982#discussion_r975880160

ota-meshi avatar Sep 20 '22 23:09 ota-meshi

~~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 avatar Sep 20 '22 23:09 ota-meshi

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

ota-meshi avatar Sep 20 '22 23:09 ota-meshi

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

dev1437 avatar Sep 23 '22 13:09 dev1437