Skip to content

fix: support HH:MM am/pm time formats in combined date-time parsing#284

Open
0xSoftBoi wants to merge 2 commits intouutils:mainfrom
0xSoftBoi:fix/hhmm-ampm-parsing
Open

fix: support HH:MM am/pm time formats in combined date-time parsing#284
0xSoftBoi wants to merge 2 commits intouutils:mainfrom
0xSoftBoi:fix/hhmm-ampm-parsing

Conversation

@0xSoftBoi
Copy link
Copy Markdown

Summary

  • The combined date-time parser (combined::parse) used time::iso which 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" orphaned
  • Switched to time::parse (which tries am_pm_time first with fallback to iso), so am/pm suffixes are now correctly consumed as part of the time
  • Case insensitivity is already handled by parse_items lowercasing input before parsing

Fixes #282

Test plan

  • Added test date_and_time_ampm covering all three failing inputs from the issue: "2024-06-15 12:00 PM", "2024-06-15 11:30am", "2024-06-15 3:00 PM"
  • All 376 existing tests pass (cargo test)
  • No regressions — am_pm_time fails gracefully (backtrack) on inputs without meridiem, falling through to iso

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>
@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq bot commented Apr 6, 2026

Merging this PR will degrade performance by 3.93%

❌ 2 regressed benchmarks
✅ 19 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
parse_weekday 30 µs 31.2 µs -3.93%
parse_extended_year_relative 30.9 µs 32.2 µs -3.89%

Comparing 0xSoftBoi:fix/hhmm-ampm-parsing (fdef5ec) with main (7440dd9)

Open in CodSpeed

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.31%. Comparing base (7440dd9) to head (fdef5ec).

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           
Flag Coverage Δ
macos_latest 99.31% <100.00%> (+<0.01%) ⬆️
ubuntu_latest 99.31% <100.00%> (+<0.01%) ⬆️
windows_latest 14.26% <34.69%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sylvestre
Copy link
Copy Markdown
Contributor

i am surprised to see potential perf regression here. what is your take here?

@tomagsx
Copy link
Copy Markdown

tomagsx commented Apr 6, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

date input: HH:MM am/pm forms rejected (e.g. 2024-06-15 12:00 PM)

3 participants