Skip to content

fix(core): parse unambiguous day-first dates (31/12/2026) instead of leaving them as text - #1132

Closed
saadkhantareen wants to merge 1 commit into
storytold:mainfrom
saadkhantareen:fix/unambiguous-day-first-dates
Closed

saadkhantareen wants to merge 1 commit into
storytold:mainfrom
saadkhantareen:fix/unambiguous-day-first-dates

Conversation

@saadkhantareen

Copy link
Copy Markdown

Fixes #1071

Summary

Typed and pasted dates were only parsed month-first (the en-US order), so an unambiguous day-first date such as 31/12/2026 stayed plain text — even though the first field cannot be a month, and the workbook's number-format inference (OpenXML/ECMA-376) likewise only produces a date when the value is a valid calendar date.

Day-first is now tried as a fallback: only when the month-first interpretation fails valid_ymd validation. Ambiguous dates keep the existing month-first reading (e.g. 8/6/2026 still parses as August 6), and strings that are not valid calendar dates in either order (e.g. 31/4/2026) still stay text.

Changes

  • crates/core/src/parse.rs — after the existing month-first attempts in parse_date_like, try the same layouts with the first two fields swapped, each result still checked by valid_ymd; covered layouts: D/M/YYYY, D/M/YY, and D/M with the session year.
  • New unit test unambiguous_day_first_dates covering 31/12/2026, 13/6/2026, the leap day 29/2/2024, and 25/12 (session year), plus the guard cases: impossible dates stay text and ambiguous dates keep the month-first reading.

Tests

  • cargo test -p gridcraft-core — 22 passed, 0 failed, including the new test.
  • cargo fmt -p gridcraft-core -- --check — clean.

Copilot AI balanced review requested due to automatic review settings October 13, 2026 17:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@quantmind-br

Copy link
Copy Markdown
Contributor

Thanks for picking up #1071. I checked this against #447 (regional formats), which rewrites the same branch of parse_date, and found two things:

  • The example in [GridCraft] date format #1071 doesn't change. 08/06/2026 fits month/day, so with this PR it is still 6 August (parse_input returns 46240). Only inputs whose first number is 13 to 31 are read differently.
  • For someone typing day-first, the order now depends on the day: 12/06/2026 is 6 December and 13/06/2026 is 13 June. A column of such dates mixes both orders with nothing to show it.

In Excel the order comes from the regional format, and a date that doesn't fit it stays text (for example this thread, where 07/14/2023 is text under a day-first setting). #447 adds that setting (File › Options › Regional format): in en-GB, de-DE, fr-FR or pt-BR, 08/06/2026 is 8 June and 31/12/2026 is 31 December, while en-US keeps 31/12/2026 as text. So I think #1071 is better served by choosing a day-first region than by a fallback in en-US. The two PRs also conflict in crates/core/src/parse.rs.

@saadkhantareen

Copy link
Copy Markdown
Author

@quantmind-br Thank you for the careful check against #447 — you are right on both points.

The mixed-order column is the decisive one: with a fallback, 12/06/2026 and 13/06/2026 in the same column would be read in different orders with nothing to show it, and that inconsistency is worse than the date staying text. Excel's behaviour comes from the regional format, and #447 implements exactly that: under a day-first region 08/06/2026 is 8 June and 31/12/2026 is 31 December, while en-US keeps 31/12/2026 as text. That serves #1071 properly, where this PR only patched the en-US symptom — and as you note, the two also conflict in crates/core/src/parse.rs.

I am closing this PR in favour of #447. Happy to help there instead — if a reviewer or tester for the regional formats would be useful (in particular the date-parsing cases from #1071), please let me know and I will pick it up.

@quantmind-br

Copy link
Copy Markdown
Contributor

Thanks, that's generous. A review or a test pass on #447 would help, since it hasn't had one yet. For the #1071 cases: build regional-formats, pick File › Options › Regional format › English (United Kingdom), and type 08/06/2026, 31/12/2026 and 25/12. They should be 8 June, 31 December and 25 December of this year. Back in English (United States), 08/06/2026 is 6 August and 31/12/2026 stays text. Anything that reads differently from Excel in your region is worth a comment on #447.

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.

[GridCraft] date format

3 participants