Skip to content

Accept era years of three or more digits in Date._jisx0301 - #195

Open
Shaisolaris wants to merge 1 commit into
ruby:masterfrom
Shaisolaris:jisx0301-era-year
Open

Shaisolaris wants to merge 1 commit into
ruby:masterfrom
Shaisolaris:jisx0301-era-year

Conversation

@Shaisolaris

Copy link
Copy Markdown

Date#jisx0301 writes the era year without a limit on its width, so dates from 2118 on (Reiwa 100) come out with three digits, but Date._jisx0301 only accepts two digits there. The output of #jisx0301 then cannot be read back:

d = Date.new(2118, 6, 1)
d.jisx0301                 # => "R100.06.01"
Date.jisx0301(d.jisx0301)  # => Date::Error: invalid date
Date._jisx0301("R100.06.01") # => {}
Date._parse("R100.06.01")    # => {:year=>2118, :mon=>6, :mday=>1}

This change accepts three or more digits for the era year when an era initial is present. Without an initial, the year still has to be two digits, so a string like 100.05.01 is still rejected.

Tests:

  • Added cases to test__jisx0301 for R100.05.01, R100.05.01T04:05:06+09:00, 100.05.01 and a Date.jisx0301(d.jisx0301) round trip. They fail on master and pass with this change.
  • A round-trip check of iso8601, rfc3339, xmlschema, rfc2822, httpdate and jisx0301 over 100,000 random dates in years 1 to 9999 now passes for all of them. Before the change, jisx0301 failed for every date from 2118 on.
  • test/date/test_*.rb locally (Ruby 3.3.8): the failures and errors I see are the same on master and with this change (test__plus__ex, test_new_offset, and tests that need assert_warning, assert_ractor and similar helpers from ruby core that my setup does not load). I did not run the CI matrix.

Date#jisx0301 writes the era year as is, so dates from 2118 on are
written as R100.01.01 and later.  Date._jisx0301 only accepted two
digits there, so Date.jisx0301 could not read such strings back and
raised Date::Error.

Accept three or more digits when an era initial is given, as
Date._parse already does.

This branch has not been deployed

No deployments
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.

1 participant