Fix error message percentage/number precision for alpha-value-notation #4802
Conversation
possible issues with very small numbers & specifying precision
There was a problem hiding this comment.
@malsf21 Thanks for this. Looking good. @m-allanson and I looked through it together.
I (somewhat arbitrarily) chose a precision of 3 digits - is this an alright magic number? should this be user-configurable?
Let's go with 3 for now, and only add an option if it's requested
Is using
.toPrecision()the right way to go around this?
Yes, I believe so.
Should I be covering more edge cases (e.g. very large or very very small numbers)
I think we're good.
I've requested a simple change to the tests. Other than that, LGTM.
| config: ['percentage'], | ||
| fix: true, | ||
|
|
||
| reject: [ |
There was a problem hiding this comment.
Let's move these tests into the testRules above and remove the comment in favour of adding a description: .. to the test case.
We only tend to create a new testRule when the config changes or the syntax property is used.
There was a problem hiding this comment.
Yup, sounds good! I updated the PR, moving the test cases and adding a description key.
|
Updated changelog:
Thanks again @malsf21 馃憤 |
# By Mike Allanson (6) and others # Via GitHub * master: Bump @types/lodash from 4.14.152 to 4.14.154 (#4817) Update CHANGELOG.md Fix with workaround a TypeError thrown for "html" (#4797) Update CHANGELOG.md Fix false positives for variables in font-family-no-missing-generic-family-keyword (#4806) Update CHANGELOG.md Add ignoreSelectors option to block-opening-brace-space-before (#4640) Update CHANGELOG.md Fix error message percentage/number precision for alpha-value-notation (#4802) Create new 'createPartialStylelintResult' module (#4815) Move function normalizeAllRuleSettings() out to a separate module (#4810)
Hi there,
This is a PR that aims to tackle a problem addressed in #4792, namely that with floating-point conversions, the error messages (and "fixed" outputs) have numbers that are unpleasant to look at/unreasonable.
My preliminary shot at this (which might need some feedback) is to use Javascript's in-built
.toPrecision()function. I used this instead of.toFixed()as the latter rounds down very small numbers to 0 (e.g.0.003, which I included as a test case), which would create an incorrect error message. I added these precision-specifies toasNumber()andasPercentage().I am curious if this is the correct approach to solve this problem:
3digits - is this an alright magic number? should this be user-configurable?.toPrecision()the right way to go around this? I wasn't able to personally find any problems, but I imagine it might create some problems with too many trailing zeroes, or possibly truncating important dataWould love to get @jeddy3's input since they implemented the rule, or @m-allanson who originally responded to the issue!
Closes #4792.
Nothing else not in this giant wall of text of a PR 馃槃