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

Use localized date and time formats in interpolate_hash #674

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

davidgumberg
Copy link

@davidgumberg davidgumberg commented Aug 17, 2023

Interpolated Time's and Date's should respect default formatting options and locale.

Rails, for example, passes Date and Time ActiveRecord attributes to I18n::t.

(This may also avoid triggering some deprecation warnings in Rails eg: rails/rails#48960)

@radar
Copy link
Collaborator

radar commented Oct 9, 2023

@davidgumberg Would you mind adding a test for this please?

Originally disabled in
518c053 (remove timezone sensible test because it breaks on windows, 2008-06-24)

Test now works in Windows 10, Ruby 3.2.2 (via 'RubyInstaller')
Interpolated strings containing Times or Dates should respect
`:default` formatting options of locale.
@davidgumberg
Copy link
Author

@radar Thanks for the review, I've added some tests.

Is this behavior desired?

test "interpolation: given a Date with no default format set it raises I18n::MissingTranslationData" do
    assert_raises(I18n::MissingTranslationData) do
        date = Date.new(2008, 3, 1)
        interpolate(:default => '%{date}', :date => date)
    end
end

I am not really sure how much code there is in the wild that passes Date's and Time's to I18n::t, but this will break anything that does so without a :default format set for Date/Time which might be too aggressive?

The gentler alternative would be to default to sprintf as before if a format is not present.

If you have a chance, let me know what you think.

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.

None yet

2 participants