Repository navigation
fix(weekday): accept a period after abbreviated weekday names - #334
anasbekheit wants to merge 2 commits into
Conversation
|
@cakebaker I think this PR may improve GNU test parity. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #334 +/- ##
==========================================
+ Coverage 97.48% 97.49% +0.01%
==========================================
Files 21 21
Lines 4258 4279 +21
Branches 136 136
==========================================
+ Hits 4151 4172 +21
Misses 106 106
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| fn day(input: &mut &str) -> ModalResult<Day> { | ||
| s(alpha1) | ||
| s((alpha1, opt('.')).take()) |
There was a problem hiding this comment.
please add a test with the full parser, like parse_datetime("wed., 2024-01-03")
the existing "mon." unit tests already passed before, they don't check that the whole input was consumed
There was a problem hiding this comment.
Added a new commit
|
Are there any concerns blocking the merge of this PR? |
|
|
||
| #[test] | ||
| fn test_weekday_abbreviation_with_period() { | ||
|
|
There was a problem hiding this comment.
please remove this empty line
| assert_eq!(get_formatted_date(&date, weekday), expected, "{weekday}"); | ||
| } | ||
|
|
||
| let actual = crate::parse_datetime("wed., 2024-01-03") |
There was a problem hiding this comment.
2024-01-03 is already a wednesday, could you please add a case like "mon., 2024-01-03" too?
2 participants
The weekday parser matched with
alpha1, which never consumes., so the existing"mon.","tue.", … arms could never match and inputs likemon.orwed., 2024-01-03were rejected. GNUdateaccepts a period after the three-letter abbreviation. The parser now includes an optional trailing period in the word.