Add minimum_fraction_length to number_separator #1202
Conversation
bjarkehs
force-pushed
the
decimal_number_separator
branch
from
9d66167 to
ad749ac
Compare
Current coverage is 82.09% (diff: 59.25%)@@ master #1202 diff @@
==========================================
Files 165 165
Lines 8182 8201 +19
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 6724 6733 +9
- Misses 1458 1468 +10
Partials 0 0
|
| @@ -13,6 +13,11 @@ | |||
| [Aaron McTavish](https://github.com/aamctustwo) | |||
| [#1169](https://github.com/realm/SwiftLint/issues/1169) | |||
|
|
|||
| * Update `number_separator` rule to allow for specifying | |||
| minimum length of fraction. | |||
There was a problem hiding this comment.
missing two trailing whitespaces
| @@ -113,10 +113,18 @@ public struct NumberSeparatorRule: OptInRule, CorrectableRule, ConfigurationProv | |||
| return prefixes.filter { lowercased.hasPrefix($0) }.isEmpty | |||
| } | |||
|
|
|||
| private func isValid(number: String, reversed: Bool) -> (Bool, String) { | |||
| private func isValid(number: String, reversed: Bool, isFraction: Bool) -> (Bool, String) { | |||
There was a problem hiding this comment.
you can remove the reversed parameter now, since it's always equivalent to !isFraction
There was a problem hiding this comment.
Ah right, for some reason my tests wouldn't pass locally even without changing anything. So I decided not to touch it.
|
|
||
| public var consoleDescription: String { | ||
| return severityConfiguration.consoleDescription + ", minimum_length: \(minimumLength)" | ||
| let minimumFractionLengthDescription: String | ||
| if let minimumFractionLength = self.minimumFractionLength { |
There was a problem hiding this comment.
I don't think you need to use self. here
There was a problem hiding this comment.
Yeah, my bad. Code style from current project 😄
|
This seems good to me, but as we're complicating the configuration would you mind adding some tests to make sure we don't break it in the future? |
|
Yeah that sounds good. I'll give it a try. |
|
So I'm not entirely sure how to write the tests. But I added some triggering and non-triggering examples. |
I hope you don't mind, but I've pushed dc2ebdf which does some improvements. Basically:
|
|
That's perfectly fine, I'll just have a look at your commit to see how its done for the future 👍 Thanks for the help. |
|
Why doesn't this configuration item show up on the website? |
Because nothing was being included in the configuration description when the field was unset. Fixed in 54a2463. |
|
Oh, I thought that it extracts the configuration items automatically (e.g. using reflection). Thanks for the quick fix! |
This adds
minimum_fraction_lengthas a configuration tonumber_separator. If specified it overrulesminimum_length.Closes #1200