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

Ignore length-zero-no-unit violations found within var() functions if custom properties are ignored #5250

Closed
wants to merge 1 commit into from

Conversation

lanthaler
Copy link

Which issue, if any, is this issue related to?

Related to #4945, but limited to the var() function for now

Is there anything in the PR that needs further explanation?

Should be self-explanatory. More than happy to answer questions if there are any

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lanthaler Thanks for the pull request.

Two related pull requests were recently merged (#5203 & #5245) which I think will address the problem most users are facing with this rule, i.e. using var within calc or some other math function.

However, users may wish to ignore the contents of functions outright, whether that's var or something else. The convention we use is an ignore* secondary option, e.g. ignoreFunctions.

Rather than overloading the custom-properties keyword option, this option will allow users to make the rule more permissive on a granular scale.

Is it possible to rework this pull request to #4945 (comment)?

@jeddy3
Copy link
Member

jeddy3 commented Apr 21, 2021

I hadn't realised the rule was a mesh of style-search and the value parser. I've created an issue to refactor the rule to only use the value parser, which will make adding an ignoreFunction option easier.

@lanthaler
Copy link
Author

lanthaler commented Apr 22, 2021

Thanks for the review @jeddy3. I just discovered that there's already a lingering PR to add ignoreFunction #5082 and that you started refactoring this code altogether #5256. If this change isn't of interest, let's just close this PR. The reason I "overloaded" the custom-properties option is because the var function is exclusively about custom properties.

@jeddy3
Copy link
Member

jeddy3 commented Apr 23, 2021

If this change isn't of interest, let's just close this PR.

Yes, let's do that. With the recently merged #5203 & #5245 (which are in the refactoring of #5256), I think the most common problem of using var within calc is solved.

Thanks again for the pull request.

@jeddy3 jeddy3 closed this Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants