handle Count field from template data #53
Conversation
i18n/bundle/bundle.go
Outdated
| if c, ok := dataMap["Count"]; ok { | ||
| if cStr, ok := c.(string); ok { | ||
| var err error | ||
| count, err = strconv.Atoi(cStr) |
There was a problem hiding this comment.
Why is this more complicated than the original PR (#49)?
You don't need to convert the string to a number because the library already handles this conversion later on this line:
p, _ := lang.Plural(count)|
|
||
| fmt.Println(T("your_unread_email_count", map[string]interface{}{"Count": 0})) | ||
| fmt.Println(T("your_unread_email_count", map[string]interface{}{"Count": "1"})) | ||
| fmt.Println(T("your_unread_email_count", map[string]interface{}{"Count": "3.14"})) |
There was a problem hiding this comment.
Can you also add tests for "person_unread_email_count_timeframe" to show that it works with other template data?
It would also be nice to have a test for what happens if both are provided (a count parameter and a template with a Count variable).
There was a problem hiding this comment.
np, both are added.
i18n/i18n.go
Outdated
| // | ||
| // If translationID is a plural form, then the first variadic argument must be an integer type | ||
| // If translationID is a plural form, the function accepts two parameter signatures | ||
| // - T(count int, data struct{}) |
There was a problem hiding this comment.
I think this would be slightly more readable as a numbered list (i.e. 1. here instead of -).
|
The library is designed not to panic, but if consumers of the library wish to panic or do some error handling, then they can wrap the T function and detect when the returned translation is equal to the translation id. |
|
ok thanks, anyway the changes removed that err, so the question does not matter anymore |
i18n/bundle/bundle.go
Outdated
| data = dataMap | ||
| } | ||
| } | ||
| { |
There was a problem hiding this comment.
This should probably be an else right?
if count != nil {
// current logic
} else {
dataMap := toMap(data)
if c, ok := dataMap["Count"]; ok {
count = c
}
}
|
we got an else ! BTW, i just noticed the travis file may need a little update for 1.6. |
2 participants
One q i have, now there s a
strconv.Atoiif Count field value is a string.Which may return an error, see this
I m unsure if it should panic, for instance it just ignores the parameter, thus, the template rendering operation will probably fails, which, if i m right, should display the translation id, without any warning.
The "without a warning" bother me, but panicking bother me too.
:- /