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

translation file with key "id" leads LoadMessageFile to fail silently #209

Closed
davleb opened this issue Jan 10, 2020 · 3 comments · Fixed by #320
Closed

translation file with key "id" leads LoadMessageFile to fail silently #209

davleb opened this issue Jan 10, 2020 · 3 comments · Fixed by #320

Comments

@davleb
Copy link

davleb commented Jan 10, 2020

Context : every toml file with a key to translate like
"id" = "Id"

LoadMessageFile returns no error but every use of Localize function returns an empty string (like it would be if there was an error in LoadMessageFile)
The problem seems to be the identifier "id" itself. If i change it to "ed" or "ad", the problem goes away.
Is it a magic string used internally ? I use the latest V2 version of the lib.

@MJacred
Copy link
Contributor

MJacred commented Jan 10, 2020

Hi,
this is due to the implementation of func isMessage(v interface{}) bool

go-i18n/v2/i18n/message.go

Lines 193 to 204 in 26334ab

case map[string]interface{}:
for _, key := range reservedKeys {
val, ok := data[key]
if !ok {
continue
}
_, ok = val.(string)
if !ok {
continue
}
// v is a message if it contains a "reserved" key holding a string value
return true

your "id" = "Id" is identified as a reserved key and isMessage returns true and your value is not added as the translation text -> it is blank

The reserved key id is needed for json and yaml.
it would be possible to use it in toml, but isMessage is a generic function which acts the same independent of your file format. I don't recommend a change here, as it would have an unnecessary impact on readability and performance.

There's a pull request #208 pending, which would give you an error message in the terminal when you try doing what you did while running the merge command:
missing translation for message id "id"
This should suffice
@nicksnyder: #208 should be fine as-is, no?

EDIT: After thinking about this for a couple minutes, the new error message would be misleading in this edge case. Some check in extract/merge would be nice...

@davleb
Copy link
Author

davleb commented Jan 10, 2020

Hi MJacred,
Thanks for your explanation. I was not aware of the side-effect when using these keys:
"id", "description", "hash", "leftdelim", "rightdelim", "zero", "one", "two", "few", "many", "other"
Not a big deal but if any of these words is used in the toml/yaml file as a key, the bundle is not functional and Localize will return "" for every key, including valid keys. I think an error should be returned at the LoadMessageFile, as it is the case when a key is defined more than one time in the toml/yaml file:
Near line 14 (last key parsed 'Validate changes'): Key 'Validate changes' has already been defined.

@jeremy-serenne
Copy link

I just encountered this issue, it was hard to find this topic as I got the problem with the key "other".

Even if it's a 2 years old issue, do you know when there will be an error returned by LoadMessageFile and LoadMessageFileFS. I really think it would be very useful for debugging 🙏 (@MJacred? I don't know who to ping 😅)

nicksnyder added a commit that referenced this issue Oct 13, 2024
Makes it an error to provide data that mixes reserved message keys with
unreserved ones. Previously the unreserved keys would be ignored, but
this caused surprising behavior where if someone added a message that
had a key with a reserved word, it would silently break the whole
translation file. This change turns that situation into an error.

Resolves #209
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 a pull request may close this issue.

3 participants