Options framework to allow more control over parsing #89
Conversation
Credit for this to KamalGalrani
Created PreferMonthFirst option, which exposes preferMonthFirst in a backwards compatible manner with optional spread functions that operate on the parser when it's being constructed
…er features that may have options
Original retry logic by KamalGalrani Adapted into a deferred function at the source enabled by an option, by me
Update RetryAmbiguousDateWithSwap detection logic after testing
|
CI is failing due to unrelated issue #90. |
Add comment for ParserOption
Codecov Report
@@ Coverage Diff @@
## master #89 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 1 1
Lines 899 929 +30
=====================================
+ Hits 899 929 +30
Continue to review full report at Codecov.
|
|
This now also fixes #90 by vendoring github.com/apcera/termtables |
|
@araddon Any thoughts? |
|
Dates with dots as separators (like preferMonthFirst := dateparse.PreferMonthFirst(false)
t, err := dateparse.ParseAny("30.1.2014", preferMonthFirst) // => parsing time "30.1.2014": month out of range
if err != nil {
fmt.Println(err)
}
t, err = dateparse.ParseAny("30/1/2014", preferMonthFirst) // => 2014-01-30 00:00:00 +0000 UTC
if err != nil {
fmt.Println(err)
}
fmt.Println(t) |
|
That's due to an inconsistency in how this package labels dates as ambiguous. This flag is only set and returned as the ambiguous date error in the case from the test. If the dot case set ambiguousMMDD, then the option would activate. |
|
Sounds like an issue in general. Do you want to open an issue? I will if you don't want to. |
|
Sure, you can open one, and to elaborate a bit more, it's the check for preferMonthFirst, and alternate logic if it's false which will allow this test (and any similar case with different overall format) to pass; setting ambiguousMMDD will allow the retry with opposite preference option to activate. |
|
I'd love to use this package, but I can't without this feature. Anything holding it up? |
Codecov Report
@@ Coverage Diff @@
## master #89 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 899 929 +30
=========================================
+ Hits 899 929 +30
Continue to review full report at Codecov.
|
Not from me. I just removed the vendoring that was made unnecessary by #101. |
|
@araddon is there a reason this isn't pulled into master? |
|
Thank you! |
This PR creates a framework for creating and applying arbitrary options without breaking changes. It follows the pattern used in github.com/gorilla/handlers. Options are created by implementing the ParserOption interface. They are then passed in a spread to existing parsing functions. The options are passed down into the parser constructor, where they are then applied on the parser. Through the excellent architecture already present in this package, the changes to accomplish this are straightforward.
The first options to implement the ParserOption interface are PreferMonthFirst and RetryAmbiguousDateWithSwap.
PreferMonthFirst exposes parser.preferMonthFirst, and the code handling the false case was added by #88.
RetryAmbiguousDateWithSwap adds a new parser field, parser.retryAmbiguousDateWithSwap, allowing parsing not to fail when the month is not actually ambiguous because if it were swapped with the day it would be outside the range of valid month values. It does this by checking if the month was ambiguous; if so, then trying to parse, and rebuilding the parser with the opposite preferMonthFirst value and disabling retryAmbiguousDateWithSwap. The code of the logic behind the option was added by #88, and this PR places its implementation in a much tighter scope while also exposing it as a ParserOption.