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

Add missing gettext calls in core_components.ex #5658

Merged
merged 3 commits into from
Dec 27, 2023

Conversation

rhcarvalho
Copy link
Contributor

@rhcarvalho rhcarvalho commented Dec 4, 2023

Closes #5649.

Note: strings in app.html.heex and home.html.heex in the generated app are not "meant to stay", so, after ignoring them, the only thing left in a freshly generated app was core_components.ex, which already had 2 strings marked for translation -- the rest is updated in this PR.


Depending on the context of the template file, we need to output code with different syntax. To make maintenance easier and keep the with/without Gettext strings consistent, we add a helpers maybe_*_gettext/2 to contextually call gettext or skip it, while outputting correct syntax. The helpers are internal and are never exposed to the generated code.

Both installer and Phoenix (through phx.gen.live) have a core_components.ex template file, and those must stay in sync. We don't want to create a dependency between Phoenix and the installer, so the helpers are implemented in both places.

Depending on the context of the template file, we need to output code
with different syntaxes. To make maintenance easier and keep the
with/without Gettext strings consistent, we add a helper
`maybe_gettext/3` to contextually call gettext or skip it, while
outputting correct syntax. The helper is internal and is never exposed
to the generated code.

Both installer and Phoenix (through phx.gen.live) have a
core_components.ex template file, and those must stay in sync. We don't
want to create a dependency between Phoenix and the installer, so
`maybe_gettext/3` is implemented in both places and, similar to the
core_components.ex template itself, we ensure that `maybe_gettext/3`
implementations stay in sync with automated tests.
And define those functions as private functions in existing modules,
as per code review feedback.
else
message
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of defining it inside the quote, define them as regular functions inside PhxNew.Generator which will be imported.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Phx.New.Generator is always imported, so we can define the functions
just once.
@josevalim josevalim merged commit 561369b into phoenixframework:main Dec 27, 2023
4 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

1 similar comment
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

agonzalezro pushed a commit to agonzalezro/phoenix that referenced this pull request Dec 30, 2023
studzien pushed a commit to Whatnot-Inc/phoenix that referenced this pull request Feb 8, 2024
studzien pushed a commit to Whatnot-Inc/phoenix that referenced this pull request Feb 16, 2024
studzien pushed a commit to Whatnot-Inc/phoenix that referenced this pull request Feb 20, 2024
@rhcarvalho rhcarvalho deleted the phx_new-gettext branch May 14, 2024 07:57
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.

Strings in generated apps are not fully covered by gettext
2 participants