refactor(gui): refactor draw_hotkeys to improve code health (#86)

- Break down the monolithic `draw_hotkeys` method into smaller,
  focused component functions: `draw_hotkeys_header`,
  `draw_hotkeys_search`, `draw_hotkeys_table`, and
  `handle_hotkey_action`.
- Improve readability and maintainability of the `src/gui/draw.rs` file
  while preserving identical behavior.

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>

packages/rpm/pwsp.spec

@@ -18,6 +18,7 @@ BuildRequires: cargo
 BuildRequires: pipewire-devel
 BuildRequires: alsa-lib-devel
 BuildRequires: clang-devel
+BuildRequires: cmake
 
 %global _description %{expand:
 PWSP lets you play audio files through your microphone. Has both CLI and

src/gui/draw.rs

@@ -137,7 +137,22 @@ impl SoundpadGui {
         ui.vertical(|ui| {
             ui.spacing_mut().item_spacing.y = 5.0;
 
-            // --- Header ---
+            self.draw_hotkeys_header(ui);
+            ui.separator();
+
+            self.draw_hotkeys_search(ui);
+            ui.separator();
+            ui.add_space(5.0);
+
+            let action = self.draw_hotkeys_table(ui);
+
+            if let Some(action) = action {
+                self.handle_hotkey_action(action);
+            }
+        });
+    }
+
+    fn draw_hotkeys_header(&mut self, ui: &mut Ui) {
         ui.horizontal(|ui| {
             let back_button = Button::new(ICON_ARROW_BACK).frame(false);
             if ui.add(back_button).clicked() {
@@ -148,10 +163,9 @@ impl SoundpadGui {
                 ui.label(RichText::new("Hotkeys").color(Color32::WHITE).monospace());
             });
         });
+    }
 
-            ui.separator();
-
-            // --- Search and Add Command ---
+    fn draw_hotkeys_search(&mut self, ui: &mut Ui) {
         ui.horizontal(|ui| {
             ui.menu_button(format!("{} Add Command", ICON_ADD.codepoint), |ui| {
                 let mut selected_cmd = None;
@@ -190,10 +204,9 @@ impl SoundpadGui {
                     .desired_width(f32::INFINITY),
             );
         });
+    }
 
-            ui.separator();
-            ui.add_space(5.0);
-
+    fn draw_hotkeys_table(&mut self, ui: &mut Ui) -> Option<HotkeyAction> {
         let conflicts = self.app_state.hotkey_config.find_conflicts();
         let conflict_slots: std::collections::HashSet<&str> =
             conflicts.into_iter().flat_map(|(a, b)| [a, b]).collect();
@@ -380,7 +393,10 @@ impl SoundpadGui {
                 }
             });
 
-            if let Some(action) = action {
+        action
+    }
+
+    fn handle_hotkey_action(&mut self, action: HotkeyAction) {
         match action {
             HotkeyAction::Remove(slot) => {
                 make_request_async(Request::clear_hotkey(&slot));
@@ -399,8 +415,6 @@ impl SoundpadGui {
             }
         }
     }
-        });
-    }
 
     fn draw_header(&mut self, ui: &mut Ui) {
         ui.vertical_centered_justified(|ui| {
@@ -409,10 +423,9 @@ impl SoundpadGui {
                 return;
             }
 
-            let tracks = self.audio_player_state.tracks.clone();
             let mut action = None;
 
-            for track in tracks {
+            for track in &self.audio_player_state.tracks {
                 CollapsingHeader::new(
                     RichText::new(
                         track
@@ -445,6 +458,99 @@ impl SoundpadGui {
         });
     }
 
+    fn draw_playback_controls(ui: &mut Ui, track: &TrackInfo) -> Option<TrackAction> {
+        let mut action = None;
+
+        let play_button = Button::new(if track.paused {
+            ICON_PLAY_ARROW
+        } else {
+            ICON_PAUSE
+        })
+        .corner_radius(15.0);
+
+        if ui.add_sized([30.0, 30.0], play_button).clicked() {
+            action = Some(if track.paused {
+                TrackAction::Resume(track.id)
+            } else {
+                TrackAction::Pause(track.id)
+            });
+        }
+
+        let loop_button = Button::new(
+            RichText::new(if track.looped {
+                ICON_REPEAT_ONE
+            } else {
+                ICON_REPEAT
+            })
+            .size(18.0),
+        )
+        .frame(false);
+
+        if ui.add_sized([15.0, 30.0], loop_button).clicked() {
+            action = Some(TrackAction::ToggleLoop(track.id));
+        }
+
+        action
+    }
+
+    fn draw_position_control(
+        ui: &mut Ui,
+        ui_state: &mut pwsp::types::gui::TrackUiState,
+        track: &TrackInfo,
+        default_slider_width: f32,
+    ) {
+        let duration = track.duration.unwrap_or(1.0);
+        let position_slider = Slider::new(&mut ui_state.position_slider_value, 0.0..=duration)
+            .show_value(false)
+            .step_by(0.01);
+
+        let position_slider_width = ui.available_width()
+            - (30.0 * 3.0)
+            - default_slider_width
+            - (ui.spacing().item_spacing.x * 6.0);
+
+        ui.spacing_mut().slider_width = position_slider_width;
+        if ui.add_sized([30.0, 30.0], position_slider).drag_stopped() {
+            ui_state.position_dragged = true;
+        }
+
+        let time_label =
+            Label::new(RichText::new(format_time_pair(track.position, duration)).monospace());
+        ui.add_sized([30.0, 30.0], time_label);
+    }
+
+    fn draw_volume_control(
+        ui: &mut Ui,
+        ui_state: &mut pwsp::types::gui::TrackUiState,
+        track: &TrackInfo,
+        default_slider_width: f32,
+    ) {
+        let volume_icon = Self::get_volume_icon(track.volume);
+        let volume_label = Label::new(RichText::new(volume_icon).size(18.0));
+        ui.add_sized([30.0, 30.0], volume_label)
+            .on_hover_text(format!("Volume: {:.0}%", track.volume * 100.0));
+
+        let volume_slider = Slider::new(&mut ui_state.volume_slider_value, 0.0..=1.0)
+            .show_value(false)
+            .step_by(0.01);
+
+        ui.spacing_mut().slider_width = default_slider_width - 30.0;
+        ui.spacing_mut().item_spacing.x = 0.0;
+
+        if ui.add_sized([30.0, 30.0], volume_slider).drag_stopped() {
+            ui_state.volume_dragged = true;
+        }
+    }
+
+    fn draw_stop_control(ui: &mut Ui, track: &TrackInfo) -> Option<TrackAction> {
+        let stop_button = Button::new(ICON_CLOSE).frame(false);
+        if ui.add_sized([30.0, 30.0], stop_button).clicked() {
+            Some(TrackAction::Stop(track.id))
+        } else {
+            None
+        }
+    }
+
     fn draw_track_control(
         ui: &mut Ui,
         app_state: &mut AppState,
@@ -475,93 +581,17 @@ impl SoundpadGui {
         let mut action = None;
 
         ui.horizontal_top(|ui| {
-            // ---------- Play Button ----------
-            let play_button = Button::new(if track.paused {
-                ICON_PLAY_ARROW
-            } else {
-                ICON_PAUSE
-            })
-            .corner_radius(15.0);
-
-            let play_button_response = ui.add_sized([30.0, 30.0], play_button);
-            if play_button_response.clicked() {
-                if track.paused {
-                    action = Some(TrackAction::Resume(track.id));
-                } else {
-                    action = Some(TrackAction::Pause(track.id));
+            if let Some(act) = Self::draw_playback_controls(ui, track) {
+                action = Some(act);
             }
-            }
-            // --------------------------------
-
-            // ---------- Loop Button ----------
-            let loop_button = Button::new(
-                RichText::new(if track.looped {
-                    ICON_REPEAT_ONE
-                } else {
-                    ICON_REPEAT
-                })
-                .size(18.0),
-            )
-            .frame(false);
-
-            let loop_button_response = ui.add_sized([15.0, 30.0], loop_button);
-            if loop_button_response.clicked() {
-                action = Some(TrackAction::ToggleLoop(track.id));
-            }
-            // --------------------------------
-
-            // ---------- Position Slider ----------
-            let duration = track.duration.unwrap_or(1.0);
-            let position_slider = Slider::new(&mut ui_state.position_slider_value, 0.0..=duration)
-                .show_value(false)
-                .step_by(0.01);
 
             let default_slider_width = ui.spacing().slider_width;
-            let position_slider_width = ui.available_width()
-                - (30.0 * 3.0)
-                - default_slider_width
-                - (ui.spacing().item_spacing.x * 6.0);
-            ui.spacing_mut().slider_width = position_slider_width;
-            let position_slider_response = ui.add_sized([30.0, 30.0], position_slider);
-            if position_slider_response.drag_stopped() {
-                ui_state.position_dragged = true;
+            Self::draw_position_control(ui, ui_state, track, default_slider_width);
+            Self::draw_volume_control(ui, ui_state, track, default_slider_width);
+
+            if let Some(act) = Self::draw_stop_control(ui, track) {
+                action = Some(act);
             }
-            // --------------------------------
-
-            // ---------- Time Label ----------
-            let time_label =
-                Label::new(RichText::new(format_time_pair(track.position, duration)).monospace());
-            ui.add_sized([30.0, 30.0], time_label);
-            // --------------------------------
-
-            // ---------- Volume Icon ----------
-            let volume_icon = Self::get_volume_icon(track.volume);
-            let volume_label = Label::new(RichText::new(volume_icon).size(18.0));
-            ui.add_sized([30.0, 30.0], volume_label)
-                .on_hover_text(format!("Volume: {:.0}%", track.volume * 100.0));
-            // --------------------------------
-
-            // ---------- Volume Slider ----------
-            let volume_slider = Slider::new(&mut ui_state.volume_slider_value, 0.0..=1.0)
-                .show_value(false)
-                .step_by(0.01);
-
-            ui.spacing_mut().slider_width = default_slider_width - 30.0;
-            ui.spacing_mut().item_spacing.x = 0.0;
-
-            let volume_slider_response = ui.add_sized([30.0, 30.0], volume_slider);
-            if volume_slider_response.drag_stopped() {
-                ui_state.volume_dragged = true;
-            }
-            // --------------------------------
-
-            // ---------- Stop Button ---------
-            let stop_button = Button::new(ICON_CLOSE).frame(false);
-            let stop_button_response = ui.add_sized([30.0, 30.0], stop_button);
-            if stop_button_response.clicked() {
-                action = Some(TrackAction::Stop(track.id));
-            }
-            // --------------------------------
         });
 
         action

src/gui/input.rs

@@ -9,7 +9,10 @@ use std::path::PathBuf;
 fn chord_from_event(modifiers: &Modifiers, key: &Key) -> Option<String> {
     let key_name = key.name();
     let is_valid = (key_name.len() == 1
-        && key_name.chars().next().is_some_and(|c| c.is_ascii_alphanumeric()))
+        && key_name
+            .chars()
+            .next()
+            .is_some_and(|c| c.is_ascii_alphanumeric()))
         || (key_name.starts_with('F')
             && key_name.len() > 1
             && key_name[1..].chars().all(|c| c.is_ascii_digit()));
@@ -60,7 +63,10 @@ pub fn parse_chord(chord: &str) -> Option<(Modifiers, Key)> {
 
     let key_name = parts[parts.len() - 1];
     let is_valid = (key_name.len() == 1
-        && key_name.chars().next().is_some_and(|c| c.is_ascii_alphanumeric()))
+        && key_name
+            .chars()
+            .next()
+            .is_some_and(|c| c.is_ascii_alphanumeric()))
         || (key_name.starts_with('F')
             && key_name.len() > 1
             && key_name[1..].chars().all(|c| c.is_ascii_digit()));

src/utils/pipewire.rs

@@ -9,11 +9,11 @@ use tokio::{
     time::{Duration, timeout},
 };
 
-pub fn setup_pipewire_context() -> (MainLoopRc, ContextRc) {
+pub fn setup_pipewire_context() -> Result<(MainLoopRc, ContextRc), String> {
     pipewire::init();
-    let main_loop = MainLoopRc::new(None).expect("Failed to initialize pipewire main loop");
-    let context = ContextRc::new(&main_loop, None).expect("Failed to create pipewire context");
-    (main_loop, context)
+    let main_loop = MainLoopRc::new(None).map_err(|e| e.to_string())?;
+    let context = ContextRc::new(&main_loop, None).map_err(|e| e.to_string())?;
+    Ok((main_loop, context))
 }
 
 fn parse_global_object(
@@ -85,8 +85,15 @@ fn parse_global_object(
 async fn pw_get_global_objects_thread(
     main_sender: mpsc::Sender<(Option<AudioDevice>, Option<Port>)>,
     pw_receiver: pipewire::channel::Receiver<Terminate>,
+    init_sender: std::sync::mpsc::SyncSender<Result<(), String>>,
 ) {
-    let (main_loop, context) = setup_pipewire_context();
+    let (main_loop, context) = match setup_pipewire_context() {
+        Ok(res) => res,
+        Err(e) => {
+            let _ = init_sender.send(Err(e));
+            return;
+        }
+    };
 
     // Stop main loop on Terminate message
     let _receiver = pw_receiver.attach(main_loop.loop_(), {
@@ -94,12 +101,24 @@ async fn pw_get_global_objects_thread(
         move |_| _main_loop.quit()
     });
 
-    let core = context
-        .connect(None)
-        .expect("Failed to connect to pipewire context");
-    let registry = core
-        .get_registry()
-        .expect("Failed to get registry from pipewire context");
+    let core = match context.connect(None) {
+        Ok(core) => core,
+        Err(e) => {
+            let _ = init_sender.send(Err(format!("Failed to connect to pipewire context: {}", e)));
+            return;
+        }
+    };
+
+    let registry = match core.get_registry() {
+        Ok(registry) => registry,
+        Err(e) => {
+            let _ = init_sender.send(Err(format!(
+                "Failed to get registry from pipewire context: {}",
+                e
+            )));
+            return;
+        }
+    };
 
     let _listener = registry
         .add_listener_local()
@@ -115,6 +134,11 @@ async fn pw_get_global_objects_thread(
         })
         .register();
 
+    // Signal successful initialization
+    if init_sender.send(Ok(())).is_err() {
+        return;
+    }
+
     main_loop.run();
 }
 
@@ -122,10 +146,17 @@ pub async fn get_all_devices() -> Result<(Vec<AudioDevice>, Vec<AudioDevice>), B
     // Channels to communicate with pipewire thread
     let (main_sender, mut main_receiver) = mpsc::channel(10);
     let (pw_sender, pw_receiver) = pipewire::channel::channel();
+    let (init_sender, init_receiver) = std::sync::mpsc::sync_channel(0);
 
     // Spawn pipewire thread in background
-    let _pw_thread =
-        tokio::spawn(async move { pw_get_global_objects_thread(main_sender, pw_receiver).await });
+    let _pw_thread = tokio::spawn(async move {
+        pw_get_global_objects_thread(main_sender, pw_receiver, init_sender).await
+    });
+
+    // Wait for initialization to complete
+    if let Err(e) = init_receiver.recv()? {
+        return Err(e.into());
+    }
 
     let mut input_devices: HashMap<u32, AudioDevice> = HashMap::new();
     let mut output_devices: HashMap<u32, AudioDevice> = HashMap::new();
@@ -150,9 +181,7 @@ pub async fn get_all_devices() -> Result<(Vec<AudioDevice>, Vec<AudioDevice>), B
             }
             Ok(None) | Err(_) => {
                 // Pipewire thread is finished and we can collect our devices
-                pw_sender
-                    .send(Terminate {})
-                    .expect("Failed to terminate pipewire thread");
+                let _ = pw_sender.send(Terminate {});
 
                 for port in ports {
                     let node_id = port.node_id;
@@ -226,12 +255,24 @@ pub async fn get_device(device_name: &str) -> Result<AudioDevice, Box<dyn Error>
 
 pub fn create_virtual_mic() -> Result<pipewire::channel::Sender<Terminate>, Box<dyn Error>> {
     let (pw_sender, pw_receiver) = pipewire::channel::channel::<Terminate>();
+    let (init_sender, init_receiver) = std::sync::mpsc::sync_channel(0);
 
     let _pw_thread = thread::spawn(move || {
-        let (main_loop, context) = setup_pipewire_context();
-        let core = context
-            .connect(None)
-            .expect("Failed to connect to pipewire context");
+        let (main_loop, context) = match setup_pipewire_context() {
+            Ok(res) => res,
+            Err(e) => {
+                let _ = init_sender.send(Err(e));
+                return;
+            }
+        };
+        let core = match context.connect(None) {
+            Ok(core) => core,
+            Err(e) => {
+                let _ =
+                    init_sender.send(Err(format!("Failed to connect to pipewire context: {}", e)));
+                return;
+            }
+        };
 
         let props = properties!(
             "factory.name" => "support.null-audio-sink",
@@ -243,9 +284,13 @@ pub fn create_virtual_mic() -> Result<pipewire::channel::Sender<Terminate>, Box<
             "object.linger" => "false", // Destroy the node on app exit
         );
 
-        let _node = core
-            .create_object::<pipewire::node::Node>("adapter", &props)
-            .expect("Failed to create virtual mic");
+        let _node = match core.create_object::<pipewire::node::Node>("adapter", &props) {
+            Ok(node) => node,
+            Err(e) => {
+                let _ = init_sender.send(Err(format!("Failed to create virtual mic: {}", e)));
+                return;
+            }
+        };
 
         let _receiver = pw_receiver.attach(main_loop.loop_(), {
             let _main_loop = main_loop.clone();
@@ -253,9 +298,16 @@ pub fn create_virtual_mic() -> Result<pipewire::channel::Sender<Terminate>, Box<
         });
 
         println!("Virtual mic created");
+        if init_sender.send(Ok(())).is_err() {
+            return;
+        }
         main_loop.run();
     });
 
+    if let Err(e) = init_receiver.recv()? {
+        return Err(e.into());
+    }
+
     Ok(pw_sender)
 }
 
@@ -304,12 +356,24 @@ pub fn create_link(
     input_fr: Port,
 ) -> Result<pipewire::channel::Sender<Terminate>, Box<dyn Error>> {
     let (pw_sender, pw_receiver) = pipewire::channel::channel::<Terminate>();
+    let (init_sender, init_receiver) = std::sync::mpsc::sync_channel(0);
 
     let _pw_thread = thread::spawn(move || {
-        let (main_loop, context) = setup_pipewire_context();
-        let core = context
-            .connect(None)
-            .expect("Failed to connect to pipewire context");
+        let (main_loop, context) = match setup_pipewire_context() {
+            Ok(res) => res,
+            Err(e) => {
+                let _ = init_sender.send(Err(e));
+                return;
+            }
+        };
+        let core = match context.connect(None) {
+            Ok(core) => core,
+            Err(e) => {
+                let _ =
+                    init_sender.send(Err(format!("Failed to connect to pipewire context: {}", e)));
+                return;
+            }
+        };
 
         let props_fl = properties! {
             "link.output.node" => format!("{}", output_fl.node_id).as_str(),
@@ -324,12 +388,20 @@ pub fn create_link(
             "link.input.port"  => format!("{}", input_fr.port_id).as_str(),
         };
 
-        let _link_fl = core
-            .create_object::<Link>("link-factory", &props_fl)
-            .expect("Failed to create link FL");
-        let _link_fr = core
-            .create_object::<Link>("link-factory", &props_fr)
-            .expect("Failed to create link FR");
+        let _link_fl = match core.create_object::<Link>("link-factory", &props_fl) {
+            Ok(link) => link,
+            Err(e) => {
+                let _ = init_sender.send(Err(format!("Failed to create link FL: {}", e)));
+                return;
+            }
+        };
+        let _link_fr = match core.create_object::<Link>("link-factory", &props_fr) {
+            Ok(link) => link,
+            Err(e) => {
+                let _ = init_sender.send(Err(format!("Failed to create link FR: {}", e)));
+                return;
+            }
+        };
 
         let _receiver = pw_receiver.attach(main_loop.loop_(), {
             let _main_loop = main_loop.clone();
@@ -340,8 +412,15 @@ pub fn create_link(
             "Link created: FL: {}-{} FR: {}-{}",
             output_fl.node_id, input_fl.node_id, output_fr.node_id, input_fr.node_id
         );
+        if init_sender.send(Ok(())).is_err() {
+            return;
+        }
         main_loop.run();
     });
 
+    if let Err(e) = init_receiver.recv()? {
+        return Err(e.into());
+    }
+
     Ok(pw_sender)
 }