Consider adding gettext functions that accept locale as argument

[lexmag]

Right now it requires extra work and affects performance (e.g. the usage of Gettext.with_locale/3).
An use case for it is the construction of push notification messages.

[josevalim]

I think we already have those they are just not exposed (but you probably knew that).

[whatyouhide]

This is also kind of a "natural" function to have as internally other functions would still be built on it, e.g.:

defmodule Gettext do
  def gettext(backend, msgid, bindings \\ %{}) do
    backend.lgettext(get_locale(backend), msgid, bindings)
  end
end

would become just

defmodule Gettext do
  def gettext(backend, msgid, bindings) do
    gettext(backend, get_locale(locale), msgid, bindings)
  end
end

(roughly ofc)

The only nasty thing I foresee (but which is an implementation detail) is that arities will start clashing, e.g., gettext(backend, locale, msgid) vs gettext(backend, msgid, bindings).

[whatyouhide]

@josevalim yes we have them as l*gettext in the backends, but we don't expose them in the backend or in Gettext.

[josevalim]

@whatyouhide right. I would consider exposing them instead so we avoid the argument issue.

[whatyouhide]

@josevalim expose the l*gettext functions in the backends you mean? What about macros, should they support specifying a locale as well?

[josevalim]

@whatyouhide because it is meant to be dynamic, no need to have them in the backend only on Gettext?

[whatyouhide]

The locale is meant to be dynamic but the msgids not necessarily, right? I mean in our use case we would call:

MyApp.Gettext.lgettext(user_locale, "This guy scored a goal!")

and still use mix gettext.extract.

[josevalim]

@whatyouhide good call, yes.

[whatyouhide]

@josevalim so the thing will be that it will be nasty to generate private and internal l*gettext/* functions and l*gettext/* public macros in a backend IMO. What I would do is rename the internal functions to something like compiled_gettext and compiled_ngettext and introduce the public macros lgettext, lngettext, ldgettext, ldngettext (jeez these names are horrible btw, ldngettext is a mouthful). Wdyt?

[josevalim]

@whatyouhide another option is to allow it only through Gettext and in the dynamic format... or just accept Gettext.with_locale is the way to go because of the API complexity.

[whatyouhide]

@josevalim the thing is that as I said above I think it would be pretty legit to use lgettext with a compile-time string that you then want extracted by gettext.extract 😕 As for the with_locale, the problem is that it's indeed quite slower I think because we need to set some app env, retrieve it to translate, and set it back after. But I agree on the complexity that the API would gain so I'm not sure what a good solution would be. Thoughts @lexmag?

[josevalim]

Don't we use the process dictionary?

[whatyouhide]

Correct, sorry, I meant the pdict :)

[lexmag]

Yeah, I think neither with_locale (besides pdict dance there is anonymous function creation) nor dynamic functions are good options, it feels to me a nice solution to try in that comment: #112 (comment).
Otherwise we would need to fork Gettext and adjust it to our needs. 😞

[josevalim]

Have you measured? Because storing to pdict and a anonymous function sounds
really really minor, specially because you are going over the network for
push notifications anyway?

On Monday, July 25, 2016, Aleksei Magusev notifications@github.com wrote:

Yeah, I think neither with_locale (besides pdict dance there is anonymous
function creation) nor dynamic functions are good options, it feels to me a
nice solution to try in that comment: #112 (comment)
#112 (comment)
.
Otherwise we would need to fork Gettext and adjust it to our needs. 😞

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#112 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAlbjHdfEq81kSDuaM7RpCOU96YxBd0ks5qZJKlgaJpZM4JRusM
.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

[lexmag]

Yes, indeed it's tiny difference for a single call, but not having anonymous function creation it makes payload generating about 2 times faster, for 1 million recipients – translation calls take 0,09s instead of 0,15s.
It may do some greater difference when several push events happening at the same time, also there is unnecessary garbage.
That said, not sure this justifies the change (at lest we don't need to fork, not today ). ¯_(ツ)_/¯

[whatyouhide]

I'm torn a bit on this. On one hand, these macros seem really natural to have and they also bring improvements in terms of speed (slight) and garbage (with the anon function, this could get nastier than speed). However, on the other hand, we had no other users requesting this, which makes me think it's not a features many want; note that this may just be because users are ok with with_locale/2 and are not too worried (either because they don't know or don't care) about pdict or anonymous functions.

What do you suggest we do here? :)

[josevalim]

In your case, can't you just set the locale without with_locale? What is
the point of wrapping in function if you are changing it in the next item
anyway?

On Monday, July 25, 2016, Andrea Leopardi notifications@github.com wrote:

I'm torn a bit on this. On one hand, these macros seem really natural to
have and they also bring improvements in terms of speed (slight) and
garbage (with the anon function, this could get nastier than speed).
However, on the other hand, we had no other users requesting this, which
makes me think it's not a features many want; note that this may just be
because users are ok with with_locale/2 and are not too worried (either
because they don't know or don't care) about pdict or anonymous functions.

What do you suggest we do here? :)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#112 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAlbj3_7pG-EpzEfkEDIkJy6EY_j_sKks5qZQE2gaJpZM4JRusM
.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

[lexmag]

We probably can and likely will do (don't really remember why we didn't do it from the beginning), anyway, I think these new locale functions can wait. 💛

[whatyouhide]

Awesome, let's wait if other users request this, especially now that they may see this issue. Thanks everybody for the help 💟