[EN|ES|PT|CN DateTimeV2] Relative ranges in a year not always correctly recognized (#2651) #2770
Conversation
|
fix implemented in CN too |
aitelint
changed the title
|
|
||
| var month = this.config.MonthOfYear[monthStr]; | ||
| var month = match.Groups["month"].Success ? this.config.MonthOfYear[monthStr] : |
There was a problem hiding this comment.
Can you add a code comment here to make it easier to understand?
| @@ -1732,6 +1734,49 @@ private DateTimeResolutionResult ParseDuration(string text, DateObject reference | |||
| endDate = modAndDateResult.EndDate; | |||
| } | |||
|
|
|||
| // Handle cases like "first 2 weeks of 2019", "last 3 months of this year" | |||
| var matchBefore = this.config.FirstLastRegex.Match(beforeStr); | |||
There was a problem hiding this comment.
Is there any kind of "pre-check" that can happen here to avoid applying both regexes always to parsing? Or is this already nested somewhere that isn't called frequently?
There was a problem hiding this comment.
now the 2 checks are nested
| @@ -291,6 +296,63 @@ public int GetYearFromText(Match match) | |||
| return year; | |||
| } | |||
|
|
|||
| private static DateObject GetFirstThursday(int year, int month = Constants.InvalidMonth) | |||
There was a problem hiding this comment.
Shoudn't these two methods be in a common palce that can be called everywhere? The calculations are not CJK specific.
Maybe this should be in some utility in the timexlib?
There was a problem hiding this comment.
moved to DateObjectExtension
| @@ -6651,5 +6651,205 @@ | |||
| } | |||
| } | |||
| ] | |||
| }, | |||
| { | |||
| "Input": "Created On 是在 2021年的第一周", | |||
There was a problem hiding this comment.
Why do they start in English? :)
One example for code switching is OK, but it would be better if the others were fully in Chinese, and more diverse in sentence form.
There was a problem hiding this comment.
ok, test cases modified
| ] | ||
| }, | ||
| { | ||
| "Input": "Created On is in the second week of 2021", |
There was a problem hiding this comment.
Desirable: It would be great if new specs have more variety in sentence structure than just repeating the base sentence and changing the mention form. Same in other languages.
|
@aitelint Looks mostly good. Can you validate it still works and resolve the minor conflict? Thanks! |
|
still working, conflicts solved |
Fix to issue #2651 in EN, ES, PT.
Solution not yet implemented in CN.
Test cases added to DateTimeModels.