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

Interpolation Behaviour Message Format #295

Merged

Conversation

maennchen
Copy link
Member

@maennchen maennchen commented Dec 1, 2021

Implements #294

Merge #296 first

This is only a draft and not a final implementation.

It looks like it is expected to configure the extract via the mix.exs options and not via the backend. Why is that?

If we actually went this way, it would probably make more sense to yield the format when extracting. This way the extractor itself would not need to know about what format to use.

@josevalim
Copy link
Contributor

Hi @maennchen, thanks for the PR!

I believe the changes could be simpler. Change flags: MapSet.new([extracted_translations_flag]) to be the flag we got from the interpolator and call it a day? :)

@maennchen
Copy link
Member Author

@josevalim Wouldn't that mean, that no messages would be purged if extracting with another interpolator?

(Calling PO.Translations.autogenerated?)

Is it ok to use the backend config in extraction? If yes: why are there configs via the mix.exs and not only via the backend module?

@josevalim
Copy link
Contributor

The configs in mix.exs are because we don't know what are the backends until that moment, iirc.

@josevalim
Copy link
Contributor

@maennchen should we use another flag for auto generated? What does gettext use?

@maennchen
Copy link
Member Author

@josevalim I'm not aware if gettext itself does something like that. It seems like there's no (documented) flag for that: https://www.gnu.org/software/gettext/manual/html_node/PO-Files.html#index-msgstr

@maennchen
Copy link
Member Author

But I think we can just make one up as long as we prefix it. (To make sure it doesn't collide)

Something like elixir-generated.

If you would want to go down that path, I would also offer a PR and do that before this PR.

@josevalim
Copy link
Contributor

Let's go with elixir-generated, yeah. 👍

@maennchen maennchen marked this pull request as draft December 6, 2021 14:59
@maennchen maennchen force-pushed the interpolation_gettext_format branch 2 times, most recently from 495cb34 to cb53d85 Compare December 6, 2021 15:06
@maennchen maennchen marked this pull request as ready for review December 6, 2021 15:07
@maennchen maennchen force-pushed the interpolation_gettext_format branch 2 times, most recently from e2e0be3 to 557e4f1 Compare December 7, 2021 16:28
@maennchen maennchen force-pushed the interpolation_gettext_format branch from 557e4f1 to b36bb7e Compare December 7, 2021 18:21
@josevalim
Copy link
Contributor

Can you please rebase? And then I think this should be good!

@maennchen maennchen force-pushed the interpolation_gettext_format branch from b36bb7e to 042e696 Compare December 7, 2021 18:25
@maennchen
Copy link
Member Author

@josevalim Done

@josevalim josevalim merged commit b3315d0 into elixir-gettext:main Dec 7, 2021
@josevalim
Copy link
Contributor

💚 💙 💜 💛 ❤️

@maennchen maennchen deleted the interpolation_gettext_format branch December 7, 2021 19:17
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 this pull request may close these issues.

2 participants