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

feat: time.Format takes an optional 3rd parameter for language #10401

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gjvnq
Copy link

@gjvnq gjvnq commented Oct 27, 2022

This feature allows the time.Format template feature to take an optional 3rd parameter for the time formatting language.

{{ time.Format ":date_medium" $date }} <!-- will use the page language -->
{{ time.Format ":date_medium" $date "pt" }} <!-- will use Portuguese (pt) -->

I added this feature because of citations. I needed a way to write the dates in Portuguese even for articles in English.

@bep
Copy link
Member

bep commented Oct 28, 2022

Thanks for this. It looks mostly correct (there is a data race in there somewhere ...), but we need to think a little harder about the API before we commit to this change. Adding the language string to time.Format feels a little arbitratry.

Currently we have these language (Site) aware funcs:

  • langs.Translate
  • time.Format

(I think I got that right)

I suspect that instead of adding the language string to every relevant func (and then reimplement a cache for every variant) seems sub optimal.

I'm thinking out loud here, so please don't implement this, but I suspect it would make more sense to have a way to create a a language so you could do:

  • site.Language.Translate (which would be the same as the template func langs.Translate/i18n)
  • {{ $lang := langs.New "en" }}{{ $lang.Translate "foo bar" }}

Etc.

@gjvnq
Copy link
Author

gjvnq commented Oct 28, 2022 via email

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

Successfully merging this pull request may close these issues.

3 participants