Skip to content

Conversation

@bep
Copy link
Member

@bep bep commented Oct 18, 2025

Closes #14064

@bep bep force-pushed the fix/getridofi18nfork branch from 28d2c1f to 057b443 Compare October 19, 2025 11:12
@bep bep marked this pull request as ready for review October 19, 2025 11:12
@bep bep force-pushed the fix/getridofi18nfork branch 5 times, most recently from 602fc0e to ff41741 Compare October 19, 2025 12:45
@bep bep requested a review from Copilot October 19, 2025 13:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Migrate from the deprecated github.com/gohugoio/go-i18n/v2 to the canonical github.com/nicksnyder/go-i18n/v2 and adapt Hugo’s i18n handling accordingly.

  • Replace imports and module dependency to nicksnyder/go-i18n/v2
  • Introduce a bundleBuilder to handle unsupported/artificial language tags by remapping to real tags and parsing translation files
  • Update Translator to work with the new bundle wrapper and adjust language key handling

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.

File Description
langs/i18n/translationProvider.go Replaces direct i18n.Bundle setup with a bundleBuilder abstraction; adds logic to remap undefined/artificial language tags to real tags; updates translator initialization.
langs/i18n/i18n.go Updates Translator to work with the new internal bundle wrapper and adjusts language key normalization and logging.
go.mod Removes github.com/gohugoio/go-i18n/v2 and adds github.com/nicksnyder/go-i18n/v2 v2.6.0.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@bep
Copy link
Member Author

bep commented Oct 19, 2025

@jmooring what do you think about this PR/strategy?

@bep bep force-pushed the fix/getridofi18nfork branch 3 times, most recently from b2889ba to 463eecd Compare October 20, 2025 14:08
@jmooring
Copy link
Member

jmooring commented Oct 20, 2025

what do you think about this PR/strategy?

First, thanks for the key and private use subtag length checks.

Second, I don't fully understand (and I don't need to at the moment) the approach of using a defined (but unused) language tag when we encounter an arbitrary language tag (12345678) or an artificial language with a private use subtag (art-x-12345678). I guess the effect is that we avoid the "use of private use tags is more or less deprecated" situation as described here.

One of my concerns was how this would effect localization with arbitrary/artificial languages, but this still seems to be based on the site config key, not the language object we hijacked, so that looks good. I spot tested localization of times and currencies, as well as pluralization. I did not test collation.

But I did find one issue. Consider these translation tables:

i18n/
├── 12345678.toml
├── art-x-12345678.toml
├── de.toml
└── en.toml

If I purposely omit translating the word "foo" in each of the tables, then run hugo --printI18nWarnings, the languages of the hijacked language objects are shown instead of the language key in the site config:

WARN  i18n|MISSING_TRANSLATION|en|foo
WARN  i18n|MISSING_TRANSLATION|de|foo
WARN  i18n|MISSING_TRANSLATION|ka|foo <-- not sure which one this is 
WARN  i18n|MISSING_TRANSLATION|ur|foo <-- not sure which one this is 

If art-x-12345678.toml uses a map for the key "foo", and the map doesn't contain a key named "other", the resulting warning is also incorrect:

Failed to get translated string for language "ur" and ID "foo": message "foo" has no plural form "other"
                                              --
@bep
Copy link
Member Author

bep commented Oct 20, 2025

One of my concerns was how this would effect localization with arbitrary/artificial languages, but this still seems to be based on the site config key, not the language object we hijacked, so that looks good.

Yea, this is the i18n func only. I will fix the error warning messages, good catch.

The above isn't the best workaround in the world, but the fact that I had forgot that we had forked the go-i18n library tells me that we needed to fix this, and it probably also makes an argument about deprecate the use of "fake" language codes in the future.

@bep bep force-pushed the fix/getridofi18nfork branch from 463eecd to ffc2d80 Compare October 21, 2025 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants