Add support for runtime translations

[bamorim]

Closes #280.

Since I don't think it makes sense to use defoverridable if this is meant to be part of the core, I changed from using super to just renaming the actual compile-time implementation functions to lgettext_compiled and lngettext_compiled and then just wrapping the call to that from lgettext/lngettext in different ways, depending on whether repo is defined or not.

[josevalim]

Thank you! I will review the PR with more detail later. For now I just want to say that the ETS repo should not be part of Gettext. We will need to define a repository for tests though in the test helper, but that can likely be done with something simpler, otherwise ETS or agent.

[bamorim]

I removed the ETS repo from here. I agree it probably makes sense to not be included.

Thanks <3

[josevalim]

Looking like a great start. With this, I'm thinking we can probably create a Gettext.CompiledRepo and shove all the precompiled translations in there, right?

We had a discussion along this line, but the issue is that the compiled repo needs to do specific compile time behavior at completion time. So we would actually need to define a repo module per backend at compilation time and I don't think that's worth it.

@bamorim, we should probably make the repo configuration be {repo, arg}, so we can do stuff like configuring the ETS table name. Or alternatively a {mod, fun, args}. Any preferences @whatyouhide?

[bamorim]

@whatyouhide as @josevalim mentioned, that would be a big change so I don't think it is worth right now. I particularly think having a "CompiledRepo" being generated "looks more clean", but it would be a lot of changes. Also, falling back to the compile time is important, so that would mean we need multiple repos, which adds a little bit to the complexity.

One way I can see us going on the route of multiple repos + compile time repo is to later add the :repos option which by default would be something like:

repos = case {opts[:repos], opts[:repo]} do
  {nil, nil} -> [CompileTimeRepo]
  {nil, repo} -> [CompileTimeRepo, repo]
  {repos, _} -> repos
end

That would give us time to think whether this CompileTimeRepo is actually worth and introduce the idea in a backwards-compatible way.

@josevalim as for the repo receiving an argument, I was thinking about that when implementing the test. It might be good to have that, but would that mean we also need something like repo.init (alike Plug)?

The problem with {mod, fun, args} is that currently we have two different methods for plural vs non plural (and they have different arities because plural needs to pass the plural form), but this could be circumvented by having something like:

  @type translation_id() ::
          {:singular, locale(), domain(), msgctxt(), msgid()}
          | {:plural, locale(), domain(), msgctxt(), msgid(), plural_form()}

So that the repo is just a /2 function.

Taking inspiration from Plug, we could even make so that repo: :get_translation is just a call to mybackend.get_translation(id, opts) or something like that.

[josevalim]

{mod, arg} with init sounds good to me then!

[bamorim]

Just an update on that. Last weekend I couldn't find time to work more on that. Will try again this weekend.

[bamorim]

@josevalim I've made the suggested change I was in doubt whether to call init in compile time or runtime, so I'll leave up to discussion. For now I'm calling at compile time following how Plug normally works. The downside is that this now there is a compile-time dependency between the Gettext backend and the repo, but I think this is okay. It also opens the possibility of maybe, in the future, making the compilation of the po files in that init callback, for example and maybe moving the default behavior to a repo itself.

[josevalim]

As long as the repository is passed at compilation time, Then it is fine to call init at compile time.

[bamorim]

I think I'm done here. Is there anything missing? Is this something we would like to move forward with?

Also, thanks for all the help <3

[bamorim]

@josevalim @whatyouhide Hey, sorry to bother you.

Is there anything that you would like to see here that is missing? Would you like to try a different approach? I could try something different if needed.

[josevalim]

Unfortunately I picked up a hand injury which makes my contribution time quite limited. So I won't be able to take this forward. Sorry :-(

[bamorim]

Hey, that is sad @josevalim. Wishing you a fast recovery. Anytime you would like just ping me here and I can get back at it, for now recovering is more important. <3

[jc00ke]

I can see this feature being of great value to us soon, so if there's anything I can do to help out, please let me know. I hope your hand is healed up by now @josevalim! ⚕️ ✋

[whatyouhide]

@bamorim tests seem to be failing? 🤔

[bamorim]

@whatyouhide some changes to the test fixutres made that happened. I rebased it and fixed the tests now.

[luka-TU]

@bamorim hi! is this update still going to happen? the feature looks cool and is very needed :)

[bamorim]

@luka-TU sorry, I've been struggling with some aspects of my life recently but I do plan on trying to fix/update the comments of the review here. Sorry for that

[luka-TU]

@bamorim no need to apologize! Hope everything is better now. I just had similar request and then found out this cool PR :) Let me know if I could be of help.

[coveralls]

Pull Request Test Coverage Report for Build cb5aabb893ffe35e0fa36ebcc00351fb2e1fd57d-PR-305

• 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.07%) to 90.669%

---

Totals
Change from base Build e5ba0651805b3b777b0018ce276e950521dab18f:	0.07%
Covered Lines:	515
Relevant Lines:	568

---

💛 - Coveralls

[szsoppa]

Hey guys, we built an open-source tool based on this feature (https://github.com/curiosum-dev/kanta). Can I help somehow to finish this PR? :)

[bamorim]

@szsoppa I think the pending discussion was around responsibilities as discussed here.

If it was up to me, I'd go with the sane defaults approach. If people think this is a good idea, I think it should take me an afternoon to implement that code.

[kipcole9]

I'm curious if there is still an intention to finish this up and merge?

[bamorim]

Hey @kipcole9 , sorry, I took a time away from any OSS contribution and public speaking because I was no in the best state of mind. Id love to be able to wrap it up. I need to get back to the pending discussions to understand what is missing.

[kipcole9]

No need to be sorry at all!

I think it's a valuable contribution but everyone contributing OSS has to balance a lot of priorities so I understand your challenge.

Thanks for making such a big effort already.

[vitalis]

👍🏻