Closed golightlyb closed 1 year ago
Hi @golightlyb
Thank you very much for your contribution! Also sorry for the late response.
I've been thinking on this use case, and I think you have a valid use case, but I'm not so sure on the approach. This are the points I don't like much:
// ...
str := "Some string to translate"
tr, exists := l.GetE(str)
if !exists {
log.Printf("Translation not found: %s", str)
// ... or whatever you want to do here
}
Which is OK, but isn't pretty much the same as this?
// ...
str := "Some string to translate"
tr := l.Get(str)
if tr == str {
log.Printf("Translation not found: %s", str)
// ... or whatever you want to do here
}
Doesn't that accomplish the same? I understand it can be trickier once you start using variables, domains, contexts, etc. But it's potentially viable. Let me know if I'm missing something here as well.
Please let me know your thoughts, I'd like to talk more about your use case and try to figure out the best way to find a solution for you and everybody else using the package.
Thanks for those comments. I agree with your points and had the same sort of feeling that it wasn't ideal to have the new methods, and an error return value might be the better option. I appreciate your careful consideration!
str := "Some string to translate" tr := l.Get(str) if tr == str {
This actually works for me, but if the translation key was equal to a valid translation (e.g. in English) then this isn't enough, you'd also have to compare that the locale wasn't the locale that the original translation key was written in too. That isn't as bad as it sounds because I only have to implement this check once in my template function. But as a general purpose solution it would be nice to solve that properly eventually, maybe in another major version.
Thanks and best wishes
Hi, I've been using this workaround, but its a bit awkward.
Having to repeat variables (and a second Sprintf call, which is extra work)
// ...
str := "Hello %s!"
tr := l.Get(str, "Bob")
if tr == fmt.Sprintf(str, "Bob") {
log.Printf("Translation not found: %s", str)
// ... or whatever you want to do here
}
It's also easy to get this wrong by mistake if the arguments don't line up exactly. This is hard to spot because it's only for the case where the format string is missing, not the general happy path.
And for singular/plural N versions of functions, I'm doing things like this: (which is actually wrong, because I'm not even handling arguments)
exists := ((n != 1) && (result != singular)) || ((n != 1) && (result != plural))
For these reasons I've continued using the fork with the pull request in my codebase. gotext
is a great lib, I just need this feature.
If its of interest, in addition to saving missing strings to a log, I am also doing this to dynamically support fallback languages (e.g. CY with a fallback to en-GB, fr-BE with a fallback to fr-FR, etc.)
Hi, thank you very much for your feedback. I understand completely.
If you're interested, I can take a different PR, to start what would be the v2 of the package, including the interface change for the Translator interface methods, to start returning the error response we discussed before. At this point, after your feedback, I think it's something that would be nice to have in the right way.
We could release a v2.0 beta or something with the initial change so it's usable and vendor-able for your use cases. And from there I can keep working on other backward-compatibility-breaking changes that I may also introduce.
Following Go Modules design, the v2 version can be a completely different codebase than v1 and progress can be made on both versions in parallel.
Sounds good to me. I'm very grateful for the positive repsonse and will get back to you with a PR shortly. Best wishes.
Currently, a translation that doesn't exist just defaults to the passed message ID.
It can be helpful to be able to catch these missing cases e.g. to save to a log, so it might be helpful to have variants of the functions that return a boolean "ok" or an error or something
Here's a PR #41 that tries to implement this.
What do you think of this approach?