From 030f8c62a6e7d5a9459a33faf54d89da398e2660 Mon Sep 17 00:00:00 2001 From: nevaforget Date: Tue, 2 Jun 2026 14:31:31 +0200 Subject: [PATCH] refactor: power-confirm via PowerAction table (v0.10.1) Replace the two hand-wired reboot/shutdown handlers and the loose-param show_power_confirm with a PowerAction table + create_power_button factory, mirroring moonset's ActionDef pattern. Couples icon/prompt/error/action so a mismatched prompt/action pair is unrepresentable. Restore the in-flight re-trigger guard via power_box.set_sensitive(false) (re-enabled on failure), superseding the v0.10.0 "no guard" tradeoff. --- Cargo.lock | 2 +- Cargo.toml | 2 +- DECISIONS.md | 7 +++ src/greeter.rs | 155 +++++++++++++++++++++++++++++++------------------ 4 files changed, 106 insertions(+), 60 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b151485..a9092e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -575,7 +575,7 @@ dependencies = [ [[package]] name = "moongreet" -version = "0.10.0" +version = "0.10.1" dependencies = [ "gdk-pixbuf", "gdk4", diff --git a/Cargo.toml b/Cargo.toml index f9ed760..c1db339 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "moongreet" -version = "0.10.0" +version = "0.10.1" edition = "2024" description = "A greetd greeter for Wayland with GTK4 and Layer Shell" license = "MIT" diff --git a/DECISIONS.md b/DECISIONS.md index fc74d32..b70f43e 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1,5 +1,12 @@ # Decisions +## 2026-06-02 – Align power-confirm to moonset's ActionDef pattern (v0.10.1) + +- **Who**: ClaudeCode, Dom +- **Why**: Code review of v0.10.0 flagged the power-confirm code (ported verbatim from moonlock) as lower-altitude than moonset's: two near-identical reboot/shutdown handlers and a `show_power_confirm` taking loose `message`/`action_fn`/`error_message` params that can drift apart. moonset already solved this with an `ActionDef` table + button factory. +- **Tradeoffs**: A `PowerAction` struct + `power_actions()` table + `create_power_button` factory is slightly more machinery for just two actions, but couples icon/prompt/error/action into one value (mismatch becomes unrepresentable) and makes a third action a one-line table entry. Kept in lockstep with moonlock (same change landed there). Did NOT touch `confirm_box: Rc>>` — moonset uses the same, it is the shared convention. +- **How**: Replaced the two hand-wired handlers with a loop over `power_actions()`; `show_power_confirm`/`execute_power_action` now take `PowerAction` (Copy) instead of three loose strings. Re-introduced the in-flight re-trigger guard via `power_box.set_sensitive(false)` (re-enabled on failure) — this restores the protection that v0.10.0 dropped, superseding that entry's "no guard" tradeoff. + ## 2026-06-02 – Inline power confirmation before reboot/shutdown (v0.10.0) - **Who**: ClaudeCode, Dom diff --git a/src/greeter.rs b/src/greeter.rs index 5fadc16..c4dc2c5 100644 --- a/src/greeter.rs +++ b/src/greeter.rs @@ -472,55 +472,17 @@ pub fn create_greeter_window( power_box.set_halign(gtk::Align::End); power_box.set_valign(gtk::Align::End); - let reboot_btn = gtk::Button::new(); - reboot_btn.set_icon_name("system-reboot-symbolic"); - reboot_btn.add_css_class("power-button"); - reboot_btn.set_tooltip_text(Some(strings.reboot_tooltip)); - reboot_btn.connect_clicked(clone!( - #[weak] - confirm_area, - #[strong] - confirm_box, - #[weak] - error_label, - move |_| { - show_power_confirm( - strings.reboot_confirm, - power::reboot, - strings.reboot_failed, - strings, - &confirm_area, - &confirm_box, - &error_label, - ); - } - )); - power_box.append(&reboot_btn); - - let shutdown_btn = gtk::Button::new(); - shutdown_btn.set_icon_name("system-shutdown-symbolic"); - shutdown_btn.add_css_class("power-button"); - shutdown_btn.set_tooltip_text(Some(strings.shutdown_tooltip)); - shutdown_btn.connect_clicked(clone!( - #[weak] - confirm_area, - #[strong] - confirm_box, - #[weak] - error_label, - move |_| { - show_power_confirm( - strings.shutdown_confirm, - power::shutdown, - strings.shutdown_failed, - strings, - &confirm_area, - &confirm_box, - &error_label, - ); - } - )); - power_box.append(&shutdown_btn); + for action in power_actions() { + let button = create_power_button( + action, + strings, + &power_box, + &confirm_area, + &confirm_box, + &error_label, + ); + power_box.append(&button); + } bottom_bar.append(&power_box); overlay.add_overlay(&bottom_bar); @@ -1352,12 +1314,72 @@ fn login_worker( }) } +/// Definition for a single power-action button (reboot, shutdown). +/// Couples icon, prompt, error text and action so a button cannot be wired +/// with a mismatched prompt/action pair. Mirrors moonset's `ActionDef`. +#[derive(Clone, Copy)] +struct PowerAction { + icon_name: &'static str, + tooltip_attr: fn(&Strings) -> &'static str, + confirm_attr: fn(&Strings) -> &'static str, + error_attr: fn(&Strings) -> &'static str, + action_fn: fn() -> Result<(), PowerError>, +} + +/// The power actions offered by the greeter. +fn power_actions() -> [PowerAction; 2] { + [ + PowerAction { + icon_name: "system-reboot-symbolic", + tooltip_attr: |s| s.reboot_tooltip, + confirm_attr: |s| s.reboot_confirm, + error_attr: |s| s.reboot_failed, + action_fn: power::reboot, + }, + PowerAction { + icon_name: "system-shutdown-symbolic", + tooltip_attr: |s| s.shutdown_tooltip, + confirm_attr: |s| s.shutdown_confirm, + error_attr: |s| s.shutdown_failed, + action_fn: power::shutdown, + }, + ] +} + +/// Build a power-action icon button wired to the confirmation flow. +fn create_power_button( + action: PowerAction, + strings: &'static Strings, + power_box: >k::Box, + confirm_area: >k::Box, + confirm_box: &Rc>>, + error_label: >k::Label, +) -> gtk::Button { + let button = gtk::Button::new(); + button.set_icon_name(action.icon_name); + button.add_css_class("power-button"); + button.set_tooltip_text(Some((action.tooltip_attr)(strings))); + button.connect_clicked(clone!( + #[weak] + power_box, + #[weak] + confirm_area, + #[strong] + confirm_box, + #[weak] + error_label, + move |_| { + show_power_confirm(action, strings, &power_box, &confirm_area, &confirm_box, &error_label); + } + )); + button +} + /// Show an inline confirmation prompt before executing a power action. fn show_power_confirm( - message: &'static str, - action_fn: fn() -> Result<(), PowerError>, - error_message: &'static str, + action: PowerAction, strings: &'static Strings, + power_box: >k::Box, confirm_area: >k::Box, confirm_box: &Rc>>, error_label: >k::Label, @@ -1369,7 +1391,7 @@ fn show_power_confirm( new_box.set_halign(gtk::Align::Center); new_box.set_margin_top(16); - let confirm_label = gtk::Label::new(Some(message)); + let confirm_label = gtk::Label::new(Some((action.confirm_attr)(strings))); confirm_label.add_css_class("confirm-label"); new_box.append(&confirm_label); @@ -1379,6 +1401,8 @@ fn show_power_confirm( let yes_btn = gtk::Button::with_label(strings.confirm_yes); yes_btn.add_css_class("confirm-yes"); yes_btn.connect_clicked(clone!( + #[weak] + power_box, #[weak] confirm_area, #[strong] @@ -1386,8 +1410,7 @@ fn show_power_confirm( #[weak] error_label, move |_| { - dismiss_power_confirm(&confirm_area, &confirm_box); - execute_power_action(action_fn, error_message, &error_label); + execute_power_action(action, strings, &power_box, &confirm_area, &confirm_box, &error_label); } )); button_row.append(&yes_btn); @@ -1418,13 +1441,27 @@ fn dismiss_power_confirm(confirm_area: >k::Box, confirm_box: &Rc Result<(), PowerError>, - error_message: &'static str, + action: PowerAction, + strings: &'static Strings, + power_box: >k::Box, + confirm_area: >k::Box, + confirm_box: &Rc>>, error_label: >k::Label, ) { + dismiss_power_confirm(confirm_area, confirm_box); + + let action_fn = action.action_fn; + let error_message = (action.error_attr)(strings); + + // Desensitize the power buttons so a double-click or keyboard repeat cannot + // fire the same action twice while it is in flight. + power_box.set_sensitive(false); + glib::spawn_future_local(clone!( + #[weak] + power_box, #[weak] error_label, async move { @@ -1436,11 +1473,13 @@ fn execute_power_action( log::error!("Power action failed: {e}"); error_label.set_text(error_message); error_label.set_visible(true); + power_box.set_sensitive(true); } Err(_) => { log::error!("Power action panicked"); error_label.set_text(error_message); error_label.set_visible(true); + power_box.set_sensitive(true); } } }