fix: support HH:MM am/pm time formats in combined date-time parsing#284
fix: support HH:MM am/pm time formats in combined date-time parsing#2840xSoftBoi wants to merge 2 commits intouutils:mainfrom
Conversation
The combined date-time parser (e.g., "2024-06-15 12:00 PM") used time::iso which only accepts 24-hour notation. When an am/pm suffix followed, iso would greedily consume the time portion, leaving the meridiem orphaned and causing a parse error. Switch to time::parse which tries am_pm_time first (with fallback to iso), so inputs like "2024-06-15 12:00 PM", "2024-06-15 11:30am", and "2024-06-15 3:00 PM" are now accepted. Case insensitivity is already handled by parse_items lowercasing the input before parsing. Fixes uutils#282 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merging this PR will degrade performance by 3.93%
Performance Changes
Comparing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #284 +/- ##
=======================================
Coverage 99.30% 99.31%
=======================================
Files 20 20
Lines 3894 3941 +47
Branches 122 127 +5
=======================================
+ Hits 3867 3914 +47
Misses 26 26
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
i am surprised to see potential perf regression here. what is your take here? |
|
Review bump on this one. Functional coverage looks good and CI is green aside from the CodSpeed regression flag. The remaining question seems to be whether the measured perf hit is acceptable for the am/pm fix, so I’d really appreciate a maintainer take on that tradeoff when you have a chance. |
Summary
combined::parse) usedtime::isowhich only accepts 24-hour notation, causing inputs like"2024-06-15 12:00 PM"to fail — the ISO parser greedily consumed"12:00"and left"pm"orphanedtime::parse(which triesam_pm_timefirst with fallback toiso), so am/pm suffixes are now correctly consumed as part of the timeparse_itemslowercasing input before parsingFixes #282
Test plan
date_and_time_ampmcovering all three failing inputs from the issue:"2024-06-15 12:00 PM","2024-06-15 11:30am","2024-06-15 3:00 PM"cargo test)am_pm_timefails gracefully (backtrack) on inputs without meridiem, falling through toiso