Add phosh gettext domain by x1z53 · Pull Request #147 · samcday/phrog

x1z53 · 2026-01-17T10:48:33Z

No description provided.

x1z53 · 2026-01-17T10:50:20Z

Attention: The translation is incomplete, as some lines do not exist in Phosh

agx · 2026-01-17T11:06:59Z

Left some initial comments, I'd maybe spin out the phosh related parts into a separate MR as I assume they can be applied right away.

agx · 2026-01-17T10:56:53Z

src/main.rs


 fn main() -> anyhow::Result<()> {
+    let _ = gettextrs::bindtextdomain("phosh", "/usr/share/locale");
+    gettextrs::textdomain("phosh")?;


? correct here as that would mean early return if gettext can't be initialized.

I would prefer the app start up with default English strings rather than not start up at all 😅

So how about:

Suggested change

gettextrs::textdomain("phosh")?;

if let Err(err) = gettextrs::textdomain("phosh") {

log::warn("onoes no i18n"...);

}

(and move this init after the following logger lines so that the log works properly :)

agx · 2026-01-17T10:57:42Z

src/lockscreen.rs

        Request::CreateSession { .. } => anyhow::Ok(Response::AuthMessage {
            auth_message_type: Secret,
-            auth_message: "Password:".into(),
+            auth_message: gettextrs::gettext("Password:").into(),


If you add new translations you need to bind another gettext domain (phrog) as otherwise the lookups will be done in phosh only (and then use dgettext("phrog", …) to look things up in the correct gettext domain.

Phosh currently uses the gettext domain for lookups but I we could switch to using gi18n-lib.h everywhere to fix the phosh gettext domain - that said using dgettext here right away doesn't have any downsides as far as I know.

The dgettext approach sounds great.

So I think this boils down to:

Suggested change

auth_message: gettextrs::gettext("Password:").into(),

auth_message: gettextrs::dgettext("phrog", "Password:").into(),

(but also note that I think clippy is complaining about this .into())

agx · 2026-01-17T10:58:23Z

src/lib.rs

 pub fn init() -> anyhow::Result<()> {
    gio::resources_register_include!("phrog.gresource").context("failed to register resources.")?;

+    let _ = gettextrs::bindtextdomain("phosh", "/usr/share/locale");


In other projects we take that path from meson but idk what's @samcday 's preference is here.

Let's just hardcode for now since this is what distros are gonna use anyway. Maybe @hustlerone could wander past here and let us know whether this would be an issue with Nix packaging. If so I'm sure (or hoping, at least) that we can still easily solve this with Cargo / build.rs and not need to bring in the Meson bazooka.

samcday · 2026-01-18T02:21:36Z

@codex review

chatgpt-codex-connector · 2026-01-18T02:23:45Z

Codex Review: Didn't find any major issues. Swish!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

samcday

Thanks for this contribution @x1z53 ... And apologies that I had not already done this minimum groundwork to support the rich existing translations from Phosh.

As Guido noted, we should bind the default domain as you've done to ensure Phosh works as expected, and then just make carefully scoped dgettext("phrog", "foo...") calls for phrog-specific bits.

As for how we juggle the translations for phrog itself, let's defer on any of that in this PR and get this landed as soon as you've fixed up the requested changes. If you want to then follow-up with some extra bits to support potfiles and their wranglin', that'd be swell 🙏

samcday · 2026-01-18T02:41:23Z

src/main.rs


 fn main() -> anyhow::Result<()> {
+    let _ = gettextrs::bindtextdomain("phosh", "/usr/share/locale");
+    gettextrs::textdomain("phosh")?;


I would prefer the app start up with default English strings rather than not start up at all 😅

So how about:

Suggested change

gettextrs::textdomain("phosh")?;

if let Err(err) = gettextrs::textdomain("phosh") {

log::warn("onoes no i18n"...);

}

(and move this init after the following logger lines so that the log works properly :)

samcday · 2026-01-18T02:43:21Z

src/lockscreen.rs

        Request::CreateSession { .. } => anyhow::Ok(Response::AuthMessage {
            auth_message_type: Secret,
-            auth_message: "Password:".into(),
+            auth_message: gettextrs::gettext("Password:").into(),


The dgettext approach sounds great.

So I think this boils down to:

Suggested change

auth_message: gettextrs::gettext("Password:").into(),

auth_message: gettextrs::dgettext("phrog", "Password:").into(),

(but also note that I think clippy is complaining about this .into())

samcday · 2026-01-18T02:44:41Z

src/lib.rs

 pub fn init() -> anyhow::Result<()> {
    gio::resources_register_include!("phrog.gresource").context("failed to register resources.")?;

+    let _ = gettextrs::bindtextdomain("phosh", "/usr/share/locale");


Let's just hardcode for now since this is what distros are gonna use anyway. Maybe @hustlerone could wander past here and let us know whether this would be an issue with Nix packaging. If so I'm sure (or hoping, at least) that we can still easily solve this with Cargo / build.rs and not need to bring in the Meson bazooka.

samcday · 2026-02-16T09:11:12Z

@codex would you please try your hand here? Open a new PR with the excellent work done here so far, and incorporate the feedback.

chatgpt-codex-connector · 2026-02-16T09:14:49Z

Summary

Added gettextrs as a dependency so we can initialize gettext and use domain-specific lookups in Rust code. Cargo.tomlL12-L23
Added shared i18n constants for the phrog domain (GETTEXT_PACKAGE) and hardcoded locale directory (LOCALEDIR) in the library module for reuse across the app. src/lib.rsL15-L17
Updated startup i18n initialization in main to:
- keep logger setup first,
- set locale,
- bind both phosh and phrog domains,
- and avoid aborting startup on gettext errors (warn instead),
- while preserving phosh as the active default textdomain. src/main.rsL27-L44
Updated the phrog-specific fake auth prompt to use dgettext("phrog", "Password:") instead of relying on the default domain, incorporating the review guidance around scoped translations. src/lockscreen.rsL347-L352
Committed the changes as 80d43f9 on the current branch and created a follow-up PR via the make_pr tool with a title/body describing these updates and test status.

Testing

✅ cargo fmt
⚠️ cargo check (failed due to environment/network restriction fetching crates index: CONNECT tunnel failed, response 403)

View task →

samcday · 2026-02-17T20:21:24Z

@x1z53 thank you very much for getting the ball rolling here. The work continues in #173 which I am intending to merge imminently. As such I will now close this PR.

x1z53 added 2 commits January 17, 2026 13:46

chore: add gettext-rs

aa8b036

i18n: init

431ab08

x1z53 changed the title ~~Add phsoh gettext domain~~ Add phosh gettext domain Jan 17, 2026

agx reviewed Jan 17, 2026

View reviewed changes

samcday added the ci-ok PR looks safe / author is trusted, run pull_request_target build workflow label Jan 18, 2026

samcday requested changes Jan 18, 2026

View reviewed changes

samcday closed this Feb 17, 2026

-    gettextrs::textdomain("phosh")?;
+    if let Err(err) = gettextrs::textdomain("phosh") {
+      log::warn("onoes no i18n"...);
+    }

	auth_message: gettextrs::gettext("Password:").into(),
	auth_message: gettextrs::dgettext("phrog", "Password:").into(),

Conversation

x1z53 commented Jan 17, 2026

Uh oh!

x1z53 commented Jan 17, 2026

Uh oh!

agx commented Jan 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samcday commented Jan 18, 2026

Uh oh!

chatgpt-codex-connector bot commented Jan 18, 2026

Uh oh!

samcday left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samcday commented Feb 16, 2026

Uh oh!

chatgpt-codex-connector bot commented Feb 16, 2026

Uh oh!

samcday commented Feb 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments