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

Add ignoreFunctions: [] to length-zero-no-unit #4945

Closed
zhawkins opened this issue Sep 16, 2020 · 4 comments
Closed

Add ignoreFunctions: [] to length-zero-no-unit #4945

zhawkins opened this issue Sep 16, 2020 · 4 comments
Labels
good first issue is good for newcomers status: ready to implement is ready to be worked on by someone type: new option a new option for an existing rule

Comments

@zhawkins
Copy link

Clearly describe the bug

The length-zero-no-unit rule ignore: ["custom-properties"] option will only run on custom property declarations like:

a { --x: 0px; }

It doesn't handle var functions like:

font: var(--x, 0px);

I see it being discussed here: #4860 (comment) but don't see a corresponding new issue so I created one. Maybe this would be considered a new feature? 🤔

Which rule, if any, is the bug related to?

length-zero-no-unit

What code is needed to reproduce the bug?

.foo {
  --x: 0px;
  margin-top: var(--y, 0px);
}

What stylelint configuration is needed to reproduce the bug?

e.g.

{
  "rules": {
    "length-zero-no-unit": [
      true,
       {
         "ignore": ["custom-properties"]
       }
    ]
  }
}

Which version of stylelint are you using?

13.6.1

How are you running stylelint: CLI, PostCSS plugin, Node.js API?

VScode plugin with settings.json

    "stylelint.validate": ["scss"],
    "editor.codeActionsOnSave": {
        "source.fixAll.stylelint": true
    }

Does the bug relate to non-standard syntax (e.g. SCSS, Less etc.)?

No.

What did you expect to happen?

I expect the unit on 0px within var(--y, 0px); to be ignored.

What actually happened (e.g. what warnings or errors did you get)?

"The following warnings were flagged:"

   Expected unit (length-zero-no-unit)
@hudochenkov hudochenkov added the status: needs discussion triage needs further discussion label Oct 1, 2020
@patric-eberle
Copy link

Same issue here. I MUST define a 0px value as fallback, but the linter triggers.

@jeddy3 jeddy3 changed the title The ignore custom-properties option for length-zero-no-unit should also ignore custom properties within var functions Add ignoreFunctions: [] to length-zero-no-unit Nov 17, 2020
@jeddy3 jeddy3 added good first issue is good for newcomers status: ready to implement is ready to be worked on by someone type: new option a new option for an existing rule and removed status: needs discussion triage needs further discussion labels Nov 17, 2020
@jeddy3
Copy link
Member

jeddy3 commented Nov 17, 2020

@zhawkins Thanks for creating the issue.

I suggest we add an ignoreFunctions: [] option for this to offer granular control to the user.

For example:

{
  "rules": {
    "length-zero-no-unit": [
      true,
       {
         "ignoreFunctions": ["var", "--custom"]
       }
    ]
  }
}

Would allow:

.foo {
  margin-top: var(--y, 0px);
  margin-bottom: --custom(0px);
}

I've labelled the issue as ready to implement. @zhawkins or @patric-eberle Please consider contributing if you have time.

There are steps on how to add a new option in the Developer guide.

@JuanFML
Copy link

JuanFML commented Dec 17, 2020

Hi! I was looking at this and I was wondering if I could try to solve this issue as my first PR?

@hudochenkov
Copy link
Member

@JuanFML of course! It would be great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue is good for newcomers status: ready to implement is ready to be worked on by someone type: new option a new option for an existing rule
Development

No branches or pull requests

5 participants