gotext icon indicating copy to clipboard operation
gotext copied to clipboard

Feature request - ways to detect missing translations at runtime

Open golightlyb opened this issue 3 years ago • 5 comments

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?

golightlyb avatar Jul 22 '20 23:07 golightlyb

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:

  • Having a new set of methods that does exactly the same but returns 1 extra response value. I'd rather refactor existing methods, but that also adds a backwards compatibility problem. We could do a major release to solve that and increase version number, but still not sure about that.
  • Returning just a bool will limit us to express only if it exists or not. I'd rather use some new error types to express when a translation does not exists or other cases as well (like domain not present or failure to retrieve or parsing the translations)
  • For your particular use case, you'll end-up having some code like this:
// ...
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.

leonelquinteros avatar Jul 27 '20 14:07 leonelquinteros

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

golightlyb avatar Jul 28 '20 05:07 golightlyb

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.)

golightlyb avatar Aug 07 '20 19:08 golightlyb

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.

leonelcarbano avatar Aug 07 '20 20:08 leonelcarbano

Sounds good to me. I'm very grateful for the positive repsonse and will get back to you with a PR shortly. Best wishes.

golightlyb avatar Aug 07 '20 23:08 golightlyb