pointfreeco/swift-parsing

Minor bug in Double parsing

gerfarfel opened this issue · 4 comments

First off, thanks for sharing this fantastic library (and all of your other work)! I ran across a small problem while working on an SVG parser. It's not at all uncommon to see leading 0's dropped in SVG path coordinates as a means of saving a few bytes.

As currently implemented the Double parser fails unless a leading digit is provided; i.e., "0.123" works, but ".123" fails to parse. The type initializer, Double(".123"), does produce the correct value, as does Double("-.123"), so fixing the parser shouldn't be difficult.

The Double parser currently looks for a leading "+" or "-", then the integer portion of the number. It fails if the integer portion is missing. Instead, it should require either the integer portion or a decimal point and fraction before failing.

Also, note that for SVG, you need to be able to parse "1.2.3" as a pair of numbers, 1.2 and 0.3. The fix for this may actually be a bit subtle.

@gerfarfel Thanks for taking the time to file this!

The behavior you describe is actually the original behavior from early on in this library's development, but we changed it to match the behavior of the Swift language's parser instead (which does not support .123).

So for better or worse, this is currently intended behavior and not a bug. You make a good point about the initializer behavior, though, so perhaps we should go back to the old style. We'll discuss soon!

Interesting. I understand the logic of matching the language parser's behavior, but there are certainly real-world scenarios where the less strict behavior is desirable – SVG being an obvious example. It would be great if the policy could be specified as an option. Thanks again for looking into this.

@gerfarfel For sure, and I think we may want to revert back to the old behavior. In the meantime you should be able to copy and paste the library implementation into your project and correct the behavior. Namely, you can remove this check:

guard !integer.isEmpty else { throw ParsingError.expectedInput(label, at: self) }

And I think you should get the behavior you're looking for!

You may also want to remove this check if you want to support omission of fractional digits:

guard !fractional.isEmpty else { return original[..<self.startIndex] }

I guess you could add your own overload that provides an option regarding whether or not the pre-decimal digits are required?