Skip to content

Commit

Permalink
langs/i18n: Fix warning regression in i18n
Browse files Browse the repository at this point in the history
Fix this by

1. Making sure that only numerical values are treated as plural counts
2. Making sure that `i18n.pluralFormNotFoundError` is not logged as a warning if `other` resolved.

Note that 2. isn't a new problem, but became visible because of the plural improvements in Hugo `0.83.0`.

Fixes #8492
  • Loading branch information
bep committed May 2, 2021
1 parent b0ca723 commit ececd1b
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 8 deletions.
24 changes: 20 additions & 4 deletions langs/i18n/i18n.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package i18n

import (
"fmt"
"reflect"
"strings"

Expand Down Expand Up @@ -89,10 +90,22 @@ func (t Translator) initFuncs(bndl *i18n.Bundle) {
PluralCount: pluralCount,
})

if err == nil && currentLang == translatedLang {
sameLang := currentLang == translatedLang

if err == nil && sameLang {
return translated
}

if err != nil && sameLang && translated != "" {
// See #8492
// TODO(bep) this needs to be improved/fixed upstream,
// but currently we get an error even if the fallback to
// "other" succeeds.
if fmt.Sprintf("%T", err) == "i18n.pluralFormNotFoundError" {
return translated
}
}

if _, ok := err.(*i18n.MessageNotFoundErr); !ok {
t.logger.Warnf("Failed to get translated string for language %q and ID %q: %s", currentLangStr, translationID, err)
}
Expand Down Expand Up @@ -120,9 +133,12 @@ func (c intCount) Count() int {
const countFieldName = "Count"

// getPluralCount gets the plural count as a string (floats) or an integer.
// If v is nil, nil is returned.
func getPluralCount(v interface{}) interface{} {
if v == nil {
return 0
// i18n called without any argument, make sure it does not
// get any plural count.
return nil
}

switch v := v.(type) {
Expand Down Expand Up @@ -171,11 +187,11 @@ func toPluralCountValue(in interface{}) interface{} {
return in
}
// A non-numeric value.
return 0
return nil
default:
if i, err := cast.ToIntE(in); err == nil {
return i
}
return 0
return nil
}
}
25 changes: 21 additions & 4 deletions langs/i18n/i18n_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,20 @@ other = "{{ . }} hours"`,
{Key: "2", Value: "2 hours"},
},
},
{
name: "Other only",
lang: "en",
id: "hour",
templ: `
[hour]
other = "{{ with . }}{{ . }}{{ end }} hours"`,
variants: []types.KeyValue{
{Key: 1, Value: "1 hours"},
{Key: "1", Value: "1 hours"},
{Key: 2, Value: "2 hours"},
{Key: nil, Value: " hours"},
},
},
{
name: "Polish",
lang: "pl",
Expand All @@ -381,13 +395,15 @@ other = "{{ . }} miesiąca"

c.Run(test.name, func(c *qt.C) {
cfg := getConfig()
cfg.Set("enableMissingTranslationPlaceholders", true)
fs := hugofs.NewMem(cfg)

err := afero.WriteFile(fs.Source, filepath.Join("i18n", test.lang+".toml"), []byte(test.templ), 0755)
c.Assert(err, qt.IsNil)

tp := NewTranslationProvider()
depsCfg := newDepsConfig(tp, cfg, fs)
depsCfg.Logger = loggers.NewWarningLogger()
d, err := deps.New(depsCfg)
c.Assert(err, qt.IsNil)
c.Assert(d.LoadResources(), qt.IsNil)
Expand All @@ -396,6 +412,7 @@ other = "{{ . }} miesiąca"

for _, variant := range test.variants {
c.Assert(f(test.id, variant.Key), qt.Equals, variant.Value, qt.Commentf("input: %v", variant.Key))
c.Assert(int(depsCfg.Logger.LogCounters().WarnCounter.Count()), qt.Equals, 0)
}

})
Expand Down Expand Up @@ -434,12 +451,12 @@ func TestGetPluralCount(t *testing.T) {
c.Assert(getPluralCount(map[string]interface{}{"Count": "32.5"}), qt.Equals, "32.5")
c.Assert(getPluralCount(map[string]interface{}{"count": 32}), qt.Equals, 32)
c.Assert(getPluralCount(map[string]interface{}{"Count": "32"}), qt.Equals, "32")
c.Assert(getPluralCount(map[string]interface{}{"Counts": 32}), qt.Equals, 0)
c.Assert(getPluralCount("foo"), qt.Equals, 0)
c.Assert(getPluralCount(map[string]interface{}{"Counts": 32}), qt.Equals, nil)
c.Assert(getPluralCount("foo"), qt.Equals, nil)
c.Assert(getPluralCount(countField{Count: 22}), qt.Equals, 22)
c.Assert(getPluralCount(countField{Count: 1.5}), qt.Equals, "1.5")
c.Assert(getPluralCount(&countField{Count: 22}), qt.Equals, 22)
c.Assert(getPluralCount(noCountField{Counts: 23}), qt.Equals, 0)
c.Assert(getPluralCount(noCountField{Counts: 23}), qt.Equals, nil)
c.Assert(getPluralCount(countMethod{}), qt.Equals, "32.5")
c.Assert(getPluralCount(&countMethod{}), qt.Equals, "32.5")

Expand All @@ -448,7 +465,7 @@ func TestGetPluralCount(t *testing.T) {
c.Assert(getPluralCount(1234.0), qt.Equals, "1234.0")
c.Assert(getPluralCount("1234"), qt.Equals, "1234")
c.Assert(getPluralCount("0.5"), qt.Equals, "0.5")
c.Assert(getPluralCount(nil), qt.Equals, 0)
c.Assert(getPluralCount(nil), qt.Equals, nil)
}

func prepareTranslationProvider(t testing.TB, test i18nTest, cfg config.Provider) *TranslationProvider {
Expand Down

0 comments on commit ececd1b

Please sign in to comment.