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

New Rule: Unnecessary Mantissa #438

Closed
Dru89 opened this issue Dec 3, 2015 · 10 comments · Fixed by #440
Closed

New Rule: Unnecessary Mantissa #438

Dru89 opened this issue Dec 3, 2015 · 10 comments · Fixed by #440

Comments

@Dru89
Copy link
Contributor

Dru89 commented Dec 3, 2015

Name: unnecessary-mantissa

Add a rule that enforces that removal of all unnecessary trailing decimal points (e.g. 4.0 should be written as 4).

I was a little ambitious and have already written the code to complete this as a7db54b but will wait to submit the pull request as per the contributing guidelines.

@Snugug
Copy link
Member

Snugug commented Dec 3, 2015

Per our rule naming conventions, if this is a strict disallow it should be named:

no-disallowed-thing

In addition, we prefer as simple language as possible, so instead of the technical mantissa I'd prefer decimal (or similar) so the rule name should be something like

no-unnecessary-decimals

Otherwise, yah, I'm ok with adding this rule.

On Dec 3, 2015, at 4:21 PM, Andrew Hays notifications@github.com wrote:

Name: unnecessary-mantissa

Add a rule that enforces that removal of all unnecessary trailing decimal points (e.g. 4.0 should be written as 4).

I was a little ambitious and have already written the code to complete this as a7db54b but will wait to submit the pull request as per the contributing guidelines.


Reply to this email directly or view it on GitHub.

@DanPurdy
Copy link
Member

DanPurdy commented Dec 3, 2015

I honestly thought no trailing zero should (and did cover this) In my head it should but yeah it actually makes sense to have this as a separate rule.

no-unnecessary-decimals would be the name i would go for too.

@Dru89
Copy link
Contributor Author

Dru89 commented Dec 3, 2015

Looking at this a bit more, it might actually make sense to modify no-trailing-zero.js to cover this use case, as they're kind of the same in my opinion.

What would you think about updating that rule, or should this be considered separate (or perhaps an option)?


Edit: Hooray! I'm really bad at catching comments, because I definitely didn't see @DanPurdy's comment above before I wrote this (as I only saw the email). But if it makes sense to have this as a separate rule, I'll definitely do that, then.

As for catching it in no-trailing-zero, it looks like the regular expression (/^(\d+\.|\.)+(\d*?[1-9])0+$/) only catches numbers that have at least one non-zero in the decimal.

@DanPurdy
Copy link
Member

DanPurdy commented Dec 4, 2015

Yeah definitely, our whole ethos with sass-lint is for small single purpose rules and I think to add this to no-trailing-zero would mean adding extra options which would be a shame.

With this as a separate rule it would allow people to enforce usage of requiring at least one zero after a decimal but not unneccessary trailing zeros after that.

And yes @Dru89 you're right, after I posted that comment last night I checked our existing rule and it does only work if you have at least one non zero number. I'll create another issue now.

Happy for you to submit a PR for your new rule though. Thanks again!

@Snugug
Copy link
Member

Snugug commented Dec 4, 2015

I see a potential lint loop here if you require the ending zero and also have no trailing zeros enabled. Thoughts Dan?

On Dec 4, 2015, at 5:10 AM, Dan Purdy notifications@github.com wrote:

Yeah definitely, our whole ethos with sass-lint is for small single purpose rules and I think to add this to no-trailing-zero would mean adding extra options which would be a shame.

With this as a separate rule it would allow people to enforce usage of requiring at least one zero after a decimal but not unneccessary trailing zeros after that.

And yes @Dru89 you're right, after I posted that comment last night I checked our existing rule and it does only work if you have at least one non zero number. I'll create another issue now.


Reply to this email directly or view it on GitHub.

@DanPurdy
Copy link
Member

DanPurdy commented Dec 4, 2015

Yeah, I think that's a possibility, maybe we should keep it in no-trailing-zero then and just have that by default enforce unnecessary 0 values after a decimal.. Would make sense, i dont think requiring the option to allow margin: 1.0 would make sense after all.

Sooooo update the rule rather than a new one?

@nirazul
Copy link

nirazul commented Dec 4, 2015

just came here to say that i love the expression "mantissa"

@Snugug
Copy link
Member

Snugug commented Dec 4, 2015

Yah, update the rule I think.

On Dec 4, 2015, at 7:29 AM, Dan Purdy notifications@github.com wrote:

Yeah, I think that's a possibility, maybe we should keep it in no-trailing-zero then and just have that by default enforce unnecessary 0 values after a decimal.. Would make sense, i dont think requiring the option to allow margin: 1.0 would make sense after all.

Sooooo update the rule rather than a new one?


Reply to this email directly or view it on GitHub.

@BPScott
Copy link

BPScott commented Dec 4, 2015

Yarp margin: 1.0 totally counts as an unwanted trailing 0 in my mind. I can't think why I'd want 'margin: 1.0' to be allowed, but 'margin: 1.00' to be invalid. Update the existing rule.

@alfaproject
Copy link

I agree with fixing the existing rule.

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.

6 participants