Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improved date localization #1535

Closed
wants to merge 2 commits into from

Conversation

rbri
Copy link
Collaborator

@rbri rbri commented Jul 22, 2024

  • use DateTimeFormatter instead of SimpleDateFormat
  • Date.prototype.toLocaleString now handles locales argument
  • make the formatter use Context.timezone instead of system default
  • ES6: use FormatStyle.SHORT for the formatter to make it behave more like real browsers when Intl.DateTimeFormat is not supported

- Date.prototype.toLocaleString now handles locales argument
- make the formatter use Context.timezone instead of system default
- ES6: use FormatStyle.SHORT for the formatter to make it behave more like real browsers when Intl.DateTimeFormat is not supported
@rbri
Copy link
Collaborator Author

rbri commented Jul 22, 2024

this is the new version of #1491

Source code is unchanged, only applied to the latest code

@rbri rbri mentioned this pull request Jul 22, 2024
@rbri
Copy link
Collaborator Author

rbri commented Jul 22, 2024

again fails with java 21 - will have a look

@gbrail
Copy link
Collaborator

gbrail commented Jul 22, 2024 via email

@rbri
Copy link
Collaborator Author

rbri commented Jul 22, 2024

Think i found it... only i18n ;-D

@rbri
Copy link
Collaborator Author

rbri commented Jul 22, 2024

@gbrail green now hope it is ok for you and can be merged

@gbrail
Copy link
Collaborator

gbrail commented Jul 22, 2024

Yes -- looks good, I will merge it.

It did introduce a few warnings from the (admittedly very pedantic) errorprone plugin. In a future PR someone should clean those up -- it just disagrees sometimes about where to use parenthesis.

Just because I'm trying to be a stickler, I'm going to add a commit to fix those warnings, and then I will merge this PR manually and close it.

One more thought -- I wonder how much simpler the whole NativeDate class would be if it used the new java.time package instead of doing its own calculations? (Plus we could use Lamba functions instead of the IdScriptableObject and it'd be even better!)

@gbrail
Copy link
Collaborator

gbrail commented Jul 22, 2024

Merged manually. Thanks!

@gbrail gbrail closed this Jul 22, 2024
@rbri rbri deleted the improved_date_localization branch July 23, 2024 06:30
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.

2 participants