Fixed format string parsing #29
Conversation
…ly prefixed with an escaped % The previous pattern would not match for "%%%foobar" even though it is a valid format string
There was a problem hiding this comment.
Hi @lvsti,
I think I understand what you're getting at, our parser isn't seeing the %foobar in %%%foobar, right?
I tried your replacement, but from what I'm seeing, it matches too much? https://regex101.com/r/Vx7Xwo/1 Namely, it matches the following:
hello 1%ld-->1%ldinstead of%ldhello %%%ld-->%%%ldinstead of%ld
|
nice catch! I simplified the regex and now it looks good for that particular case as well |
There was a problem hiding this comment.
Hi @lvsti,
Thank you for the updated commit. From some quick testing, the new regex seems to work correctly.
To finish up, could you please add a changelog entry crediting yourself (and mentioning this PR)? Please use the same format as the other entries (with . and 2 spaces at the end of the description).
CHANGELOG.md
Outdated
| @@ -6,7 +6,9 @@ | |||
|
|
|||
| ### Bug Fixes | |||
|
|
|||
| _None_ | |||
| * Improved format string regex to handle escaped "%"s that precede a variable | |||
There was a problem hiding this comment.
Please use the same format as the other entries (with . and 2 spaces at the end of the description).
| func testParseOddEscapePercentSign() { | ||
| let placeholders = StringsFileParser.PlaceholderType.placeholders(fromFormat: "%%%foo") | ||
| // Should map to [.Float] | ||
| XCTAssertEqual(placeholders, [.Float]) |
There was a problem hiding this comment.
Right, so your branch was based on an older version of SwiftGenKit. This needs to be .float instead.
The previous pattern would not match for "%%%foobar" (odd number of prefixing %'s) even though it is a valid format string.
Note that the new pattern has a slightly different behavior: the full match (i.e.
matches.rangeAt(0)) now spans all preceding %'s of the placeholder, not just one (e.g. "%%%f" instead of just "%f"). As far as I see the full match is not used anywhere in the code, still, let me know if this is a problem and I can iterate on it.