From f50ece41eb46030f91049819909fa0dd30ddf6b4 Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Fri, 10 Apr 2026 18:09:25 +0200 Subject: [PATCH 01/10] Fix all auto-fix from clippy --- src/main.rs | 101 +++++++++++++++------------------------- src/password_manager.rs | 18 +++---- src/shortcuts.rs | 5 +- src/terminal.rs | 18 +++---- src/terminal_box.rs | 26 ++++------- 5 files changed, 63 insertions(+), 105 deletions(-) diff --git a/src/main.rs b/src/main.rs index 73dd21b..e1baa39 100644 --- a/src/main.rs +++ b/src/main.rs @@ -177,11 +177,7 @@ fn main() -> Result<(), Box> { let shortcuts_config = shortcuts::ShortcutsConfig::new(config.shortcuts_custom.clone()); - let shell = if let Some(shell_program) = shell_program_opt { - Some(tty::Shell::new(shell_program, shell_args)) - } else { - None - }; + let shell = shell_program_opt.map(|shell_program| tty::Shell::new(shell_program, shell_args)); let startup_options = Some(tty::Options { shell, working_directory, @@ -554,8 +550,7 @@ impl App { .config .color_schemes(color_scheme_kind) .get(&color_scheme_id) - { - if self + && self .themes .insert( (color_scheme_name.clone(), color_scheme_kind), @@ -569,7 +564,6 @@ impl App { color_scheme_name ); } - } } } @@ -699,8 +693,8 @@ impl App { // but only for the active pane/tab if let Some(tab_model) = self.pane_model.active() { for entity in tab_model.iter() { - if tab_model.is_active(entity) { - if let Some(terminal) = tab_model.data::>(entity) { + if tab_model.is_active(entity) + && let Some(terminal) = tab_model.data::>(entity) { let mut terminal = terminal.lock().unwrap(); let current_zoom_adj = terminal.zoom_adj(); match zoom_message { @@ -714,7 +708,6 @@ impl App { } terminal.set_config(&self.config, &self.themes); } - } } } Task::none() @@ -722,8 +715,8 @@ impl App { fn save_color_schemes(&mut self, color_scheme_kind: ColorSchemeKind) -> Task { // Optimized for just saving color_schemes - if let Some(ref config_handler) = self.config_handler { - if let Err(err) = config_handler.set( + if let Some(ref config_handler) = self.config_handler + && let Err(err) = config_handler.set( match color_scheme_kind { ColorSchemeKind::Dark => "color_schemes_dark", ColorSchemeKind::Light => "color_schemes_light", @@ -732,7 +725,6 @@ impl App { ) { log::error!("failed to save config: {}", err); } - } self.update_color_schemes(); Task::none() } @@ -1026,11 +1018,10 @@ impl App { let mut found_actions = false; for action in group.actions { let action_label = shortcuts::action_label(action); - if let Some(regex) = &self.shortcut_search_regex { - if regex.find(&action_label).is_none() { + if let Some(regex) = &self.shortcut_search_regex + && regex.find(&action_label).is_none() { continue; } - } found_actions = true; let (bindings, changed) = self.shortcuts_config.bindings_for_action(action); @@ -1543,13 +1534,12 @@ impl App { self.startup_options.take().unwrap_or_default(); let options = tty::Options { shell: startup_options.shell.or_else(|| { - if let Some(mut args) = shlex::split(&profile.command) { - if !args.is_empty() { + if let Some(mut args) = shlex::split(&profile.command) + && !args.is_empty() { let command = args.remove(0); return Some(tty::Shell::new(command, args)); } - } - return None; + None }), working_directory: startup_options.working_directory.or_else( || { @@ -1956,8 +1946,8 @@ impl Application for App { .get(&color_scheme_id) .map(|color_scheme| color_scheme.name.clone()), None => Some(format!("COSMIC {:?}", color_scheme_kind)), - } { - if self.dialog_opt.is_none() { + } + && self.dialog_opt.is_none() { let (dialog, command) = Dialog::new( DialogSettings::new().kind(DialogKind::SaveFile { filename: format!("{}.ron", color_scheme_name), @@ -1974,7 +1964,6 @@ impl Application for App { self.dialog_opt = Some(dialog); return command; } - } } Message::ColorSchemeExportResult(color_scheme_kind, color_scheme_id_opt, result) => { //TODO: show errors in UI @@ -2115,8 +2104,7 @@ impl Application for App { Message::ColorSchemeRenameSubmit => { if let Some((color_scheme_kind, color_scheme_id, color_scheme_name)) = self.color_scheme_renaming.take() - { - if let Some(color_scheme) = self + && let Some(color_scheme) = self .config .color_schemes_mut(color_scheme_kind) .get_mut(&color_scheme_id) @@ -2124,7 +2112,6 @@ impl Application for App { color_scheme.name = color_scheme_name; return self.save_color_schemes(color_scheme_kind); } - } } Message::ColorSchemeTabActivate(entity) => { if let Some(color_scheme_kind) = @@ -2344,29 +2331,27 @@ impl Application for App { return self.update_focus(); } Message::FindNext => { - if !self.find_search_value.is_empty() { - if let Some(tab_model) = self.pane_model.active() { + if !self.find_search_value.is_empty() + && let Some(tab_model) = self.pane_model.active() { let entity = tab_model.active(); if let Some(terminal) = tab_model.data::>(entity) { let mut terminal = terminal.lock().unwrap(); terminal.search(&self.find_search_value, true); } } - } // Focus correct input return self.update_focus(); } Message::FindPrevious => { - if !self.find_search_value.is_empty() { - if let Some(tab_model) = self.pane_model.active() { + if !self.find_search_value.is_empty() + && let Some(tab_model) = self.pane_model.active() { let entity = tab_model.active(); if let Some(terminal) = tab_model.data::>(entity) { let mut terminal = terminal.lock().unwrap(); terminal.search(&self.find_search_value, false); } } - } // Focus correct input return self.update_focus(); @@ -2462,11 +2447,9 @@ impl Application for App { let mut terminal = terminal.lock().unwrap(); if let Some(url) = terminal.context_menu.as_ref().and_then(|m| m.link.as_ref()) - { - if let Err(err) = open::that_detached(url) { + && let Err(err) = open::that_detached(url) { log::warn!("failed to open {:?}: {}", url, err); } - } terminal.context_menu = None; terminal.active_regex_match = None; terminal.needs_update = true; @@ -2829,8 +2812,8 @@ impl Application for App { } } // Close terminal context menu state - if let Some(tab_model) = self.pane_model.active() { - if let Some(terminal) = tab_model.data::>(entity) { + if let Some(tab_model) = self.pane_model.active() + && let Some(terminal) = tab_model.data::>(entity) { let mut terminal = terminal.lock().unwrap(); //Some actions need the menu_state, //so only clear the position for them. @@ -2845,7 +2828,6 @@ impl Application for App { } } } - } tasks.push(self.update(action.message(Some(entity)))); return cosmic::Task::batch(tasks); } @@ -2967,7 +2949,7 @@ impl Application for App { .position(tab_model.active()) .and_then(|i| (i as usize).checked_sub(1)) .unwrap_or_else(|| { - tab_model.iter().count().checked_sub(1).unwrap_or_default() + tab_model.iter().count().saturating_sub(1) }); let entity = tab_model.iter().nth(pos); @@ -3007,14 +2989,13 @@ impl Application for App { } }, TermEvent::ColorRequest(index, f) => { - if let Some(tab_model) = self.pane_model.panes.get(pane) { - if let Some(terminal) = tab_model.data::>(entity) { + if let Some(tab_model) = self.pane_model.panes.get(pane) + && let Some(terminal) = tab_model.data::>(entity) { let terminal = terminal.lock().unwrap(); let rgb = terminal.colors()[index].unwrap_or_default(); let text = f(rgb); terminal.input_no_scroll(text.into_bytes()); } - } } TermEvent::CursorBlinkingChange => { //TODO: should we blink the cursor? @@ -3023,12 +3004,11 @@ impl Application for App { return self.update(Message::TabClose(Some(entity))); } TermEvent::PtyWrite(text) => { - if let Some(tab_model) = self.pane_model.panes.get(pane) { - if let Some(terminal) = tab_model.data::>(entity) { + if let Some(tab_model) = self.pane_model.panes.get(pane) + && let Some(terminal) = tab_model.data::>(entity) { let terminal = terminal.lock().unwrap(); terminal.input_no_scroll(text.into_bytes()); } - } } TermEvent::ResetTitle => { if let Some(tab_model) = self.pane_model.panes.get_mut(pane) { @@ -3047,13 +3027,12 @@ impl Application for App { return self.update_title(Some(pane)); } TermEvent::TextAreaSizeRequest(f) => { - if let Some(tab_model) = self.pane_model.panes.get(pane) { - if let Some(terminal) = tab_model.data::>(entity) { + if let Some(tab_model) = self.pane_model.panes.get(pane) + && let Some(terminal) = tab_model.data::>(entity) { let terminal = terminal.lock().unwrap(); let text = f(terminal.size().into()); terminal.input_no_scroll(text.into_bytes()); } - } } TermEvent::Title(title) => { if let Some(tab_model) = self.pane_model.panes.get_mut(pane) { @@ -3071,12 +3050,11 @@ impl Application for App { return self.update_title(Some(pane)); } TermEvent::MouseCursorDirty | TermEvent::Wakeup => { - if let Some(tab_model) = self.pane_model.panes.get(pane) { - if let Some(terminal) = tab_model.data::>(entity) { + if let Some(tab_model) = self.pane_model.panes.get(pane) + && let Some(terminal) = tab_model.data::>(entity) { let mut terminal = terminal.lock().unwrap(); terminal.needs_update = true; } - } } TermEvent::ChildExit(_error_code) => { //Ignore this for now @@ -3212,20 +3190,18 @@ impl Application for App { return self.update_config(); } Message::ContextMenuPopupClosed(id) => { - if let Some((popup_id, pane, entity, _, _, _)) = &self.context_menu_popup { - if id == *popup_id { + if let Some((popup_id, pane, entity, _, _, _)) = &self.context_menu_popup + && id == *popup_id { // Clear link underline on the terminal - if let Some(tab_model) = self.pane_model.panes.get(*pane) { - if let Some(terminal) = tab_model.data::>(*entity) { + if let Some(tab_model) = self.pane_model.panes.get(*pane) + && let Some(terminal) = tab_model.data::>(*entity) { let mut terminal = terminal.lock().unwrap(); terminal.context_menu = None; terminal.active_regex_match = None; terminal.needs_update = true; } - } self.context_menu_popup = None; } - } } Message::Surface(a) => { return cosmic::task::message(cosmic::Action::Cosmic( @@ -3334,26 +3310,23 @@ impl Application for App { } fn on_close_requested(&self, id: window::Id) -> Option { - if let Some((popup_id, _, _, _, _, _)) = &self.context_menu_popup { - if id == *popup_id { + if let Some((popup_id, _, _, _, _, _)) = &self.context_menu_popup + && id == *popup_id { return Some(Message::ContextMenuPopupClosed(id)); } - } None } fn view_window(&self, window_id: window::Id) -> Element<'_, Message> { if let Some((popup_id, _pane, entity, ref link, ref autosize_id, _)) = self.context_menu_popup - { - if window_id == popup_id { + && window_id == popup_id { return widget::autosize::autosize( menu::context_menu(&self.config, &self.key_binds, entity, link.clone()), autosize_id.clone(), ) .into(); } - } match &self.dialog_opt { Some(dialog) => dialog.view(window_id), None => widget::text("Unknown window ID").into(), diff --git a/src/password_manager.rs b/src/password_manager.rs index fdaa672..5ebecbb 100644 --- a/src/password_manager.rs +++ b/src/password_manager.rs @@ -234,11 +234,10 @@ impl PasswordManager { } // Don't do anything if nothing have changed - if let Some(original) = &original { - if original == &input_state.input { + if let Some(original) = &original + && original == &input_state.input { return Task::none(); } - } cosmic::task::future(async move { if let Err(err) = store::add_password(identifier.clone(), password.clone()).await { @@ -246,9 +245,9 @@ impl PasswordManager { "Failed to add password {identifier}: {err}" ))) } else { - if let Some(original) = original { - if original.identifier != identifier { - if let Err(err) = + if let Some(original) = original + && original.identifier != identifier + && let Err(err) = store::delete_password(original.identifier.clone()).await { return Message::PasswordManager(PasswordManagerMessage::Error( @@ -258,8 +257,6 @@ impl PasswordManager { ), )); } - } - } Message::PasswordManager(PasswordManagerMessage::None) } }) @@ -315,8 +312,8 @@ impl PasswordManager { .spacing(space_xxs), ); - if expanded { - if let Some(input_state) = &self.input_state { + if expanded + && let Some(input_state) = &self.input_state { let expanded_section: Section<'_, Message> = widget::settings::section().add( widget::column::with_children(vec![ widget::column::with_children(vec![ @@ -381,7 +378,6 @@ impl PasswordManager { passwords_section = passwords_section.add(widget::container(expanded_section).padding(padding)) } - } } sections.push(passwords_section.into()); diff --git a/src/shortcuts.rs b/src/shortcuts.rs index e918a97..aaa8897 100644 --- a/src/shortcuts.rs +++ b/src/shortcuts.rs @@ -245,12 +245,11 @@ impl ShortcutsConfig { // Remove any matching bindings return false; } - if let Some(default_action) = self.defaults.0.get(binding) { - if *default_action == reset_action { + if let Some(default_action) = self.defaults.0.get(binding) + && *default_action == reset_action { // Remove binding that overrode a default return false; } - } true }); } diff --git a/src/terminal.rs b/src/terminal.rs index 7deacfe..ce7d6d3 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -858,14 +858,12 @@ impl Terminal { } // Change color if selected - if let Some(selection) = &term.selection { - if let Some(range) = selection.to_range(&term) { - if range.contains(indexed.point) { + if let Some(selection) = &term.selection + && let Some(range) = selection.to_range(&term) + && range.contains(indexed.point) { //TODO: better handling of selection mem::swap(&mut fg, &mut bg); } - } - } // Convert foreground to linear attrs = attrs.color(fg); @@ -878,11 +876,10 @@ impl Terminal { let mut flags = indexed.cell.flags; - if let Some(active_match) = &self.active_regex_match { - if active_match.contains(&indexed.point) { + if let Some(active_match) = &self.active_regex_match + && active_match.contains(&indexed.point) { flags |= Flags::UNDERLINE; } - } if let Some(active_id) = &self.active_hyperlink_id { let mut matches_active = indexed .cell @@ -1190,14 +1187,13 @@ impl<'a, T> Iterator for HintPostProcessor<'a, T> { fn next(&mut self) -> Option { let next_match = self.next_match.take()?; - if self.start <= self.end { - if let Some(rm) = self + if self.start <= self.end + && let Some(rm) = self .term .regex_search_right(self.regex, self.start, self.end) { self.next_processed_match(rm); } - } Some(next_match) } diff --git a/src/terminal_box.rs b/src/terminal_box.rs index 8f7460e..2f22912 100644 --- a/src/terminal_box.rs +++ b/src/terminal_box.rs @@ -26,10 +26,9 @@ use cosmic::{ iced::{ Color, Element, Length, Padding, Point, Rectangle, Size, Vector, advanced::graphics::text::Raw, - event::{Event, Status}, + event::Event, keyboard::{Event as KeyEvent, Key, Modifiers}, mouse::{self, Button, Event as MouseEvent, ScrollDelta}, - window::RedrawRequest, }, theme::Theme, }; @@ -863,8 +862,8 @@ where if is_mouse_mode { state.autoscroll.stop(); } else { - if let Some((pointer, multiplier)) = state.autoscroll.next_due() { - if update_buffer_drag( + if let Some((pointer, multiplier)) = state.autoscroll.next_due() + && update_buffer_drag( state, &mut terminal, buffer_size, @@ -875,7 +874,6 @@ where ) { shell.capture_event(); } - } if state.autoscroll.is_active() { shell.request_redraw(); } @@ -1319,8 +1317,8 @@ where } Event::Mouse(MouseEvent::ButtonReleased(Button::Left)) => { state.autoscroll.stop(); - if let Some(dragging) = state.dragging.take() { - if let Dragging::Buffer { + if let Some(dragging) = state.dragging.take() + && let Dragging::Buffer { last_point, last_side, .. @@ -1334,7 +1332,6 @@ where } terminal.needs_update = true; } - } if let Some(p) = cursor_position.position_in(layout.bounds()) { let x = p.x - self.padding.left; let y = p.y - self.padding.top; @@ -1344,14 +1341,12 @@ where let location = terminal .viewport_to_point(TermPoint::new(row as usize, TermColumn(col as usize))); - if state.modifiers.control() { - if let Some(on_open_hyperlink) = &self.on_open_hyperlink { - if let Some(hyperlink) = get_hyperlink(&terminal, location) { + if state.modifiers.control() + && let Some(on_open_hyperlink) = &self.on_open_hyperlink + && let Some(hyperlink) = get_hyperlink(&terminal, location) { shell.publish(on_open_hyperlink(hyperlink)); shell.capture_event(); } - } - } if is_mouse_mode { terminal.report_mouse( @@ -1655,11 +1650,10 @@ fn update_active_regex_match( .find(|bounds| bounds.contains(&location)) { 'update: { - if let Some(active_match) = &terminal.active_regex_match { - if active_match == match_ { + if let Some(active_match) = &terminal.active_regex_match + && active_match == match_ { break 'update; } - } terminal.active_regex_match = Some(match_.clone()); terminal.needs_update = true; } From 72a27129e32d4bf8c58036e8b4d9215fae76a9d8 Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Fri, 10 Apr 2026 18:32:09 +0200 Subject: [PATCH 02/10] Don't pass the large DialogMessage on the stack Instead, since the size of DialogMessage is allocated in the Message enum, pass it as a box, so that only the size of a pointer is allocated on the stack instead. --- src/main.rs | 259 +++++++++++++++++++++++++++------------------------- 1 file changed, 137 insertions(+), 122 deletions(-) diff --git a/src/main.rs b/src/main.rs index e1baa39..2347c32 100644 --- a/src/main.rs +++ b/src/main.rs @@ -375,7 +375,7 @@ pub enum Message { DefaultFontStretch(usize), DefaultFontWeight(usize), DefaultZoomStep(usize), - DialogMessage(DialogMessage), + DialogMessage(Box), // DialogMessage is huge, so we use a box to make the size of this enum smaller on the stack Drop(Option<(pane_grid::Pane, segmented_button::Entity, DndDrop)>), Find(bool), FindNext, @@ -557,13 +557,13 @@ impl App { color_scheme.into(), ) .is_some() - { - log::warn!( - "custom {:?} color scheme {:?} replaces builtin one", - color_scheme_kind, - color_scheme_name - ); - } + { + log::warn!( + "custom {:?} color scheme {:?} replaces builtin one", + color_scheme_kind, + color_scheme_name + ); + } } } @@ -694,20 +694,21 @@ impl App { if let Some(tab_model) = self.pane_model.active() { for entity in tab_model.iter() { if tab_model.is_active(entity) - && let Some(terminal) = tab_model.data::>(entity) { - let mut terminal = terminal.lock().unwrap(); - let current_zoom_adj = terminal.zoom_adj(); - match zoom_message { - Message::ZoomIn => { - terminal.set_zoom_adj(current_zoom_adj.saturating_add(1)) - } - Message::ZoomOut => { - terminal.set_zoom_adj(current_zoom_adj.saturating_sub(1)) - } - _ => {} + && let Some(terminal) = tab_model.data::>(entity) + { + let mut terminal = terminal.lock().unwrap(); + let current_zoom_adj = terminal.zoom_adj(); + match zoom_message { + Message::ZoomIn => { + terminal.set_zoom_adj(current_zoom_adj.saturating_add(1)) } - terminal.set_config(&self.config, &self.themes); + Message::ZoomOut => { + terminal.set_zoom_adj(current_zoom_adj.saturating_sub(1)) + } + _ => {} } + terminal.set_config(&self.config, &self.themes); + } } } Task::none() @@ -722,9 +723,10 @@ impl App { ColorSchemeKind::Light => "color_schemes_light", }, self.config.color_schemes(color_scheme_kind), - ) { - log::error!("failed to save config: {}", err); - } + ) + { + log::error!("failed to save config: {}", err); + } self.update_color_schemes(); Task::none() } @@ -1019,9 +1021,10 @@ impl App { for action in group.actions { let action_label = shortcuts::action_label(action); if let Some(regex) = &self.shortcut_search_regex - && regex.find(&action_label).is_none() { - continue; - } + && regex.find(&action_label).is_none() + { + continue; + } found_actions = true; let (bindings, changed) = self.shortcuts_config.bindings_for_action(action); @@ -1535,10 +1538,11 @@ impl App { let options = tty::Options { shell: startup_options.shell.or_else(|| { if let Some(mut args) = shlex::split(&profile.command) - && !args.is_empty() { - let command = args.remove(0); - return Some(tty::Shell::new(command, args)); - } + && !args.is_empty() + { + let command = args.remove(0); + return Some(tty::Shell::new(command, args)); + } None }), working_directory: startup_options.working_directory.or_else( @@ -1946,24 +1950,24 @@ impl Application for App { .get(&color_scheme_id) .map(|color_scheme| color_scheme.name.clone()), None => Some(format!("COSMIC {:?}", color_scheme_kind)), + } && self.dialog_opt.is_none() + { + let (dialog, command) = Dialog::new( + DialogSettings::new().kind(DialogKind::SaveFile { + filename: format!("{}.ron", color_scheme_name), + }), + |msg| Message::DialogMessage(Box::new(msg)), + move |result| { + Message::ColorSchemeExportResult( + color_scheme_kind, + color_scheme_id_opt, + result, + ) + }, + ); + self.dialog_opt = Some(dialog); + return command; } - && self.dialog_opt.is_none() { - let (dialog, command) = Dialog::new( - DialogSettings::new().kind(DialogKind::SaveFile { - filename: format!("{}.ron", color_scheme_name), - }), - Message::DialogMessage, - move |result| { - Message::ColorSchemeExportResult( - color_scheme_kind, - color_scheme_id_opt, - result, - ) - }, - ); - self.dialog_opt = Some(dialog); - return command; - } } Message::ColorSchemeExportResult(color_scheme_kind, color_scheme_id_opt, result) => { //TODO: show errors in UI @@ -2050,7 +2054,7 @@ impl Application for App { self.color_scheme_errors.clear(); let (dialog, command) = Dialog::new( DialogSettings::new().kind(DialogKind::OpenMultipleFiles), - Message::DialogMessage, + |msg| Message::DialogMessage(Box::new(msg)), move |result| Message::ColorSchemeImportResult(color_scheme_kind, result), ); self.dialog_opt = Some(dialog); @@ -2108,10 +2112,10 @@ impl Application for App { .config .color_schemes_mut(color_scheme_kind) .get_mut(&color_scheme_id) - { - color_scheme.name = color_scheme_name; - return self.save_color_schemes(color_scheme_kind); - } + { + color_scheme.name = color_scheme_name; + return self.save_color_schemes(color_scheme_kind); + } } Message::ColorSchemeTabActivate(entity) => { if let Some(color_scheme_kind) = @@ -2292,7 +2296,8 @@ impl Application for App { }, Message::DialogMessage(dialog_message) => { if let Some(dialog) = &mut self.dialog_opt { - return dialog.update(dialog_message); + // DialogMessage is boxed, so we need to dereference it before updating + return dialog.update(*dialog_message); } } Message::Drop(Some((pane, entity, data))) => { @@ -2332,26 +2337,28 @@ impl Application for App { } Message::FindNext => { if !self.find_search_value.is_empty() - && let Some(tab_model) = self.pane_model.active() { - let entity = tab_model.active(); - if let Some(terminal) = tab_model.data::>(entity) { - let mut terminal = terminal.lock().unwrap(); - terminal.search(&self.find_search_value, true); - } + && let Some(tab_model) = self.pane_model.active() + { + let entity = tab_model.active(); + if let Some(terminal) = tab_model.data::>(entity) { + let mut terminal = terminal.lock().unwrap(); + terminal.search(&self.find_search_value, true); } + } // Focus correct input return self.update_focus(); } Message::FindPrevious => { if !self.find_search_value.is_empty() - && let Some(tab_model) = self.pane_model.active() { - let entity = tab_model.active(); - if let Some(terminal) = tab_model.data::>(entity) { - let mut terminal = terminal.lock().unwrap(); - terminal.search(&self.find_search_value, false); - } + && let Some(tab_model) = self.pane_model.active() + { + let entity = tab_model.active(); + if let Some(terminal) = tab_model.data::>(entity) { + let mut terminal = terminal.lock().unwrap(); + terminal.search(&self.find_search_value, false); } + } // Focus correct input return self.update_focus(); @@ -2447,9 +2454,10 @@ impl Application for App { let mut terminal = terminal.lock().unwrap(); if let Some(url) = terminal.context_menu.as_ref().and_then(|m| m.link.as_ref()) - && let Err(err) = open::that_detached(url) { - log::warn!("failed to open {:?}: {}", url, err); - } + && let Err(err) = open::that_detached(url) + { + log::warn!("failed to open {:?}: {}", url, err); + } terminal.context_menu = None; terminal.active_regex_match = None; terminal.needs_update = true; @@ -2813,21 +2821,22 @@ impl Application for App { } // Close terminal context menu state if let Some(tab_model) = self.pane_model.active() - && let Some(terminal) = tab_model.data::>(entity) { - let mut terminal = terminal.lock().unwrap(); - //Some actions need the menu_state, - //so only clear the position for them. - match action { - Action::LaunchUrlByMenu | Action::CopyUrlByMenu => { - if let Some(context_menu) = terminal.context_menu.as_mut() { - context_menu.position = None; - } - } - _ => { - terminal.context_menu = None; + && let Some(terminal) = tab_model.data::>(entity) + { + let mut terminal = terminal.lock().unwrap(); + //Some actions need the menu_state, + //so only clear the position for them. + match action { + Action::LaunchUrlByMenu | Action::CopyUrlByMenu => { + if let Some(context_menu) = terminal.context_menu.as_mut() { + context_menu.position = None; } } + _ => { + terminal.context_menu = None; + } } + } tasks.push(self.update(action.message(Some(entity)))); return cosmic::Task::batch(tasks); } @@ -2948,9 +2957,7 @@ impl Application for App { let pos = tab_model .position(tab_model.active()) .and_then(|i| (i as usize).checked_sub(1)) - .unwrap_or_else(|| { - tab_model.iter().count().saturating_sub(1) - }); + .unwrap_or_else(|| tab_model.iter().count().saturating_sub(1)); let entity = tab_model.iter().nth(pos); if let Some(entity) = entity { @@ -2990,12 +2997,13 @@ impl Application for App { }, TermEvent::ColorRequest(index, f) => { if let Some(tab_model) = self.pane_model.panes.get(pane) - && let Some(terminal) = tab_model.data::>(entity) { - let terminal = terminal.lock().unwrap(); - let rgb = terminal.colors()[index].unwrap_or_default(); - let text = f(rgb); - terminal.input_no_scroll(text.into_bytes()); - } + && let Some(terminal) = tab_model.data::>(entity) + { + let terminal = terminal.lock().unwrap(); + let rgb = terminal.colors()[index].unwrap_or_default(); + let text = f(rgb); + terminal.input_no_scroll(text.into_bytes()); + } } TermEvent::CursorBlinkingChange => { //TODO: should we blink the cursor? @@ -3005,10 +3013,11 @@ impl Application for App { } TermEvent::PtyWrite(text) => { if let Some(tab_model) = self.pane_model.panes.get(pane) - && let Some(terminal) = tab_model.data::>(entity) { - let terminal = terminal.lock().unwrap(); - terminal.input_no_scroll(text.into_bytes()); - } + && let Some(terminal) = tab_model.data::>(entity) + { + let terminal = terminal.lock().unwrap(); + terminal.input_no_scroll(text.into_bytes()); + } } TermEvent::ResetTitle => { if let Some(tab_model) = self.pane_model.panes.get_mut(pane) { @@ -3028,11 +3037,12 @@ impl Application for App { } TermEvent::TextAreaSizeRequest(f) => { if let Some(tab_model) = self.pane_model.panes.get(pane) - && let Some(terminal) = tab_model.data::>(entity) { - let terminal = terminal.lock().unwrap(); - let text = f(terminal.size().into()); - terminal.input_no_scroll(text.into_bytes()); - } + && let Some(terminal) = tab_model.data::>(entity) + { + let terminal = terminal.lock().unwrap(); + let text = f(terminal.size().into()); + terminal.input_no_scroll(text.into_bytes()); + } } TermEvent::Title(title) => { if let Some(tab_model) = self.pane_model.panes.get_mut(pane) { @@ -3051,10 +3061,11 @@ impl Application for App { } TermEvent::MouseCursorDirty | TermEvent::Wakeup => { if let Some(tab_model) = self.pane_model.panes.get(pane) - && let Some(terminal) = tab_model.data::>(entity) { - let mut terminal = terminal.lock().unwrap(); - terminal.needs_update = true; - } + && let Some(terminal) = tab_model.data::>(entity) + { + let mut terminal = terminal.lock().unwrap(); + terminal.needs_update = true; + } } TermEvent::ChildExit(_error_code) => { //Ignore this for now @@ -3191,17 +3202,19 @@ impl Application for App { } Message::ContextMenuPopupClosed(id) => { if let Some((popup_id, pane, entity, _, _, _)) = &self.context_menu_popup - && id == *popup_id { - // Clear link underline on the terminal - if let Some(tab_model) = self.pane_model.panes.get(*pane) - && let Some(terminal) = tab_model.data::>(*entity) { - let mut terminal = terminal.lock().unwrap(); - terminal.context_menu = None; - terminal.active_regex_match = None; - terminal.needs_update = true; - } - self.context_menu_popup = None; + && id == *popup_id + { + // Clear link underline on the terminal + if let Some(tab_model) = self.pane_model.panes.get(*pane) + && let Some(terminal) = tab_model.data::>(*entity) + { + let mut terminal = terminal.lock().unwrap(); + terminal.context_menu = None; + terminal.active_regex_match = None; + terminal.needs_update = true; } + self.context_menu_popup = None; + } } Message::Surface(a) => { return cosmic::task::message(cosmic::Action::Cosmic( @@ -3311,22 +3324,24 @@ impl Application for App { fn on_close_requested(&self, id: window::Id) -> Option { if let Some((popup_id, _, _, _, _, _)) = &self.context_menu_popup - && id == *popup_id { - return Some(Message::ContextMenuPopupClosed(id)); - } + && id == *popup_id + { + return Some(Message::ContextMenuPopupClosed(id)); + } None } fn view_window(&self, window_id: window::Id) -> Element<'_, Message> { if let Some((popup_id, _pane, entity, ref link, ref autosize_id, _)) = self.context_menu_popup - && window_id == popup_id { - return widget::autosize::autosize( - menu::context_menu(&self.config, &self.key_binds, entity, link.clone()), - autosize_id.clone(), - ) - .into(); - } + && window_id == popup_id + { + return widget::autosize::autosize( + menu::context_menu(&self.config, &self.key_binds, entity, link.clone()), + autosize_id.clone(), + ) + .into(); + } match &self.dialog_opt { Some(dialog) => dialog.view(window_id), None => widget::text("Unknown window ID").into(), From eca9421e87f19bd4e8af5f834a5d961c7ed9b841 Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Fri, 10 Apr 2026 18:36:16 +0200 Subject: [PATCH 03/10] We don't need to match for only one case Instead, let's use a simple if statement. If in the future we need to care about other cases, we can introduce a match statement again. --- src/main.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main.rs b/src/main.rs index 2347c32..3e815a1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -748,16 +748,16 @@ impl App { if self.find { widget::text_input::focus(self.find_search_id.clone()) } else if self.core.window.show_context { - match self.context_page { - ContextPage::KeyboardShortcuts => { - if self.shortcut_search_focus.get() { - self.shortcut_search_focus.set(false); - return widget::text_input::focus(self.shortcut_search_id.clone()); - } - } - // TODO focus for other context pages? - _ => {} + // Right now we only care about the KeyboardShortcuts context page, so we use a simple if. + // In the future if we are to care about other conext pages, we could switch this to a match + // statement instead to be cleaner. + if self.context_page == ContextPage::KeyboardShortcuts + && self.shortcut_search_focus.get() + { + self.shortcut_search_focus.set(false); + return widget::text_input::focus(self.shortcut_search_id.clone()); } + Task::none() } else if let Some(terminal_id) = self.terminal_ids.get(&self.pane_model.focused()).cloned() { From b654808734778ef53343a6990c15110b9cae9886 Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Fri, 10 Apr 2026 18:40:26 +0200 Subject: [PATCH 04/10] Ingore clippy warnings about too many arguments --- src/terminal.rs | 23 +++++++++++++---------- src/terminal_box.rs | 36 ++++++++++++++++++++---------------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/terminal.rs b/src/terminal.rs index ce7d6d3..2dd8343 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -264,6 +264,7 @@ pub struct Terminal { impl Terminal { //TODO: error handling + #[allow(clippy::too_many_arguments)] pub fn new( pane: pane_grid::Pane, entity: segmented_button::Entity, @@ -860,10 +861,11 @@ impl Terminal { // Change color if selected if let Some(selection) = &term.selection && let Some(range) = selection.to_range(&term) - && range.contains(indexed.point) { - //TODO: better handling of selection - mem::swap(&mut fg, &mut bg); - } + && range.contains(indexed.point) + { + //TODO: better handling of selection + mem::swap(&mut fg, &mut bg); + } // Convert foreground to linear attrs = attrs.color(fg); @@ -877,9 +879,10 @@ impl Terminal { let mut flags = indexed.cell.flags; if let Some(active_match) = &self.active_regex_match - && active_match.contains(&indexed.point) { - flags |= Flags::UNDERLINE; - } + && active_match.contains(&indexed.point) + { + flags |= Flags::UNDERLINE; + } if let Some(active_id) = &self.active_hyperlink_id { let mut matches_active = indexed .cell @@ -1191,9 +1194,9 @@ impl<'a, T> Iterator for HintPostProcessor<'a, T> { && let Some(rm) = self .term .regex_search_right(self.regex, self.start, self.end) - { - self.next_processed_match(rm); - } + { + self.next_processed_match(rm); + } Some(next_match) } diff --git a/src/terminal_box.rs b/src/terminal_box.rs index 2f22912..6b3a969 100644 --- a/src/terminal_box.rs +++ b/src/terminal_box.rs @@ -871,9 +871,10 @@ where layout.bounds(), self.padding, multiplier, - ) { - shell.capture_event(); - } + ) + { + shell.capture_event(); + } if state.autoscroll.is_active() { shell.request_redraw(); } @@ -1323,15 +1324,15 @@ where last_side, .. } = dragging + { { - { - let mut term = terminal.term.lock(); - if let Some(selection) = &mut term.selection { - selection.update(last_point, last_side); - } + let mut term = terminal.term.lock(); + if let Some(selection) = &mut term.selection { + selection.update(last_point, last_side); } - terminal.needs_update = true; } + terminal.needs_update = true; + } if let Some(p) = cursor_position.position_in(layout.bounds()) { let x = p.x - self.padding.left; let y = p.y - self.padding.top; @@ -1343,10 +1344,11 @@ where .viewport_to_point(TermPoint::new(row as usize, TermColumn(col as usize))); if state.modifiers.control() && let Some(on_open_hyperlink) = &self.on_open_hyperlink - && let Some(hyperlink) = get_hyperlink(&terminal, location) { - shell.publish(on_open_hyperlink(hyperlink)); - shell.capture_event(); - } + && let Some(hyperlink) = get_hyperlink(&terminal, location) + { + shell.publish(on_open_hyperlink(hyperlink)); + shell.capture_event(); + } if is_mouse_mode { terminal.report_mouse( @@ -1651,9 +1653,10 @@ fn update_active_regex_match( { 'update: { if let Some(active_match) = &terminal.active_regex_match - && active_match == match_ { - break 'update; - } + && active_match == match_ + { + break 'update; + } terminal.active_regex_match = Some(match_.clone()); terminal.needs_update = true; } @@ -1713,6 +1716,7 @@ enum EdgeScrollDirection { Bottom, } +#[allow(clippy::too_many_arguments)] fn edge_scroll_adjustment( y: f32, buffer_height: f32, From 14ec8d6760e1e93846a9b666517c3db2c7906242 Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Fri, 10 Apr 2026 18:42:34 +0200 Subject: [PATCH 05/10] Add a github action checking for clippy --- .github/workflows/rust-checks.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/rust-checks.yml b/.github/workflows/rust-checks.yml index b29c7d4..c3032c0 100644 --- a/.github/workflows/rust-checks.yml +++ b/.github/workflows/rust-checks.yml @@ -24,3 +24,18 @@ jobs: - name: Check formatting run: cargo fmt --all -- --check + + clippy: + name: Clippy + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + with: + components: clippy + + - name: Run Clippy + run: cargo clippy --all-targets --all-features -- -D warnings From a747b03c17a74c116bcda70467ebb4cc2270eb86 Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Fri, 10 Apr 2026 19:00:25 +0200 Subject: [PATCH 06/10] Bonus: reduce size of Message further Turns out we can reduce the size of Message even further by also boxing the Config variant. The size of the Message enum has now dropped from 900+ bytes to 64 bytes. --- src/main.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main.rs b/src/main.rs index 3e815a1..fbcda25 100644 --- a/src/main.rs +++ b/src/main.rs @@ -363,7 +363,7 @@ pub enum Message { ColorSchemeRename(ColorSchemeKind, ColorSchemeId, String), ColorSchemeRenameSubmit, ColorSchemeTabActivate(widget::segmented_button::Entity), - Config(Config), + Config(Box), Copy(Option), CopyOrSigint(Option), CopyPrimary(Option), @@ -2128,11 +2128,11 @@ impl Application for App { } } Message::Config(config) => { - if config != self.config { + if *config != self.config { let shortcuts_changed = config.shortcuts_custom != self.config.shortcuts_custom; log::info!("update config"); //TODO: update syntax theme by clearing tabs, only if needed - self.config = config; + self.config = *config; if shortcuts_changed { self.shortcuts_config = shortcuts::ShortcutsConfig::new(self.config.shortcuts_custom.clone()); @@ -3596,7 +3596,7 @@ impl Application for App { update.errors ); } - Message::Config(update.config) + Message::Config(Box::new(update.config)) }), match &self.dialog_opt { Some(dialog) => dialog.subscription(), From 27e2d682a507a42282052ee63ecaa7b29de3ae2f Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Fri, 10 Apr 2026 19:06:54 +0200 Subject: [PATCH 07/10] Cargo fmt --- src/password_manager.rs | 123 +++++++++++++++++++--------------------- src/shortcuts.rs | 9 +-- 2 files changed, 64 insertions(+), 68 deletions(-) diff --git a/src/password_manager.rs b/src/password_manager.rs index 5ebecbb..a52bd53 100644 --- a/src/password_manager.rs +++ b/src/password_manager.rs @@ -235,9 +235,10 @@ impl PasswordManager { // Don't do anything if nothing have changed if let Some(original) = &original - && original == &input_state.input { - return Task::none(); - } + && original == &input_state.input + { + return Task::none(); + } cosmic::task::future(async move { if let Err(err) = store::add_password(identifier.clone(), password.clone()).await { @@ -247,16 +248,13 @@ impl PasswordManager { } else { if let Some(original) = original && original.identifier != identifier - && let Err(err) = - store::delete_password(original.identifier.clone()).await - { - return Message::PasswordManager(PasswordManagerMessage::Error( - format!( - "Failed to delete password {}: {err}", - original.identifier - ), - )); - } + && let Err(err) = store::delete_password(original.identifier.clone()).await + { + return Message::PasswordManager(PasswordManagerMessage::Error(format!( + "Failed to delete password {}: {err}", + original.identifier + ))); + } Message::PasswordManager(PasswordManagerMessage::None) } }) @@ -312,72 +310,69 @@ impl PasswordManager { .spacing(space_xxs), ); - if expanded - && let Some(input_state) = &self.input_state { - let expanded_section: Section<'_, Message> = widget::settings::section().add( + if expanded && let Some(input_state) = &self.input_state { + let expanded_section: Section<'_, Message> = widget::settings::section().add( + widget::column::with_children(vec![ widget::column::with_children(vec![ - widget::column::with_children(vec![ - widget::text(fl!("password-input-description")).into(), - widget::text_input("", input_state.input.identifier.clone()) - .on_input(move |text| { - Message::PasswordManager( - PasswordManagerMessage::DescriptionInput(text), - ) - }) - .on_submit(move |text| { - Message::PasswordManager( - PasswordManagerMessage::DescriptionInputAndUpdate(text), - ) - }) - .on_unfocus(Message::PasswordManager( - PasswordManagerMessage::Update, - )) - .into(), - ]) - .spacing(space_xxxs) - .into(), - widget::column::with_children(vec![ - widget::text(fl!("password-input")).into(), - widget::secure_input( - "", - input_state.input.password.clone(), - Some(Message::PasswordManager( - PasswordManagerMessage::ToggleShowPassword, - )), - !input_state.show_password, - ) + widget::text(fl!("password-input-description")).into(), + widget::text_input("", input_state.input.identifier.clone()) .on_input(move |text| { - Message::PasswordManager(PasswordManagerMessage::PasswordInput( - text, - )) + Message::PasswordManager( + PasswordManagerMessage::DescriptionInput(text), + ) }) .on_submit(move |text| { Message::PasswordManager( - PasswordManagerMessage::PasswordInputAndUpdate(text), + PasswordManagerMessage::DescriptionInputAndUpdate(text), ) }) .on_unfocus(Message::PasswordManager( PasswordManagerMessage::Update, )) .into(), - ]) - .spacing(space_xxxs) + ]) + .spacing(space_xxxs) + .into(), + widget::column::with_children(vec![ + widget::text(fl!("password-input")).into(), + widget::secure_input( + "", + input_state.input.password.clone(), + Some(Message::PasswordManager( + PasswordManagerMessage::ToggleShowPassword, + )), + !input_state.show_password, + ) + .on_input(move |text| { + Message::PasswordManager(PasswordManagerMessage::PasswordInput( + text, + )) + }) + .on_submit(move |text| { + Message::PasswordManager( + PasswordManagerMessage::PasswordInputAndUpdate(text), + ) + }) + .on_unfocus(Message::PasswordManager(PasswordManagerMessage::Update)) .into(), ]) - .padding([0, space_s]) - .spacing(space_xs), - ); + .spacing(space_xxxs) + .into(), + ]) + .padding([0, space_s]) + .spacing(space_xs), + ); - let padding = Padding { - top: 0.0, - bottom: 0.0, - left: space_s.into(), - right: space_s.into(), - }; + let padding = Padding { + top: 0.0, + bottom: 0.0, + left: space_s.into(), + right: space_s.into(), + }; - passwords_section = - passwords_section.add(widget::container(expanded_section).padding(padding)) - } + passwords_section = + passwords_section.add(widget::container(expanded_section).padding(padding)) + } } sections.push(passwords_section.into()); diff --git a/src/shortcuts.rs b/src/shortcuts.rs index aaa8897..237ff3d 100644 --- a/src/shortcuts.rs +++ b/src/shortcuts.rs @@ -246,10 +246,11 @@ impl ShortcutsConfig { return false; } if let Some(default_action) = self.defaults.0.get(binding) - && *default_action == reset_action { - // Remove binding that overrode a default - return false; - } + && *default_action == reset_action + { + // Remove binding that overrode a default + return false; + } true }); } From 6ba1ab9e446556a8b2f85e33fefb2bf68808ac28 Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Fri, 10 Apr 2026 19:10:56 +0200 Subject: [PATCH 08/10] Update the clippy github action to install deps Unlike cargo format, clippy needs to actually build the project. --- .github/workflows/rust-checks.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/rust-checks.yml b/.github/workflows/rust-checks.yml index c3032c0..c4a8f91 100644 --- a/.github/workflows/rust-checks.yml +++ b/.github/workflows/rust-checks.yml @@ -32,6 +32,11 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 + - name: Install system dependencies + run: | + sudo apt-get update + sudo apt-get install -y pkg-config libxkbcommon-dev libfontconfig1-dev libfreetype6-dev libglvnd-dev libinput-dev libvulkan-dev libwayland-dev libx11-dev libxcursor-dev libxi-dev libxrandr-dev libasound2-dev libdbus-1-dev + - name: Install Rust uses: dtolnay/rust-toolchain@stable with: From cadbde034bded956990866a29205efd817badd57 Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Fri, 10 Apr 2026 19:23:56 +0200 Subject: [PATCH 09/10] Bonus: run the tests if we are compiling for clippy --- .github/workflows/rust-checks.yml | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust-checks.yml b/.github/workflows/rust-checks.yml index c4a8f91..f9b27b0 100644 --- a/.github/workflows/rust-checks.yml +++ b/.github/workflows/rust-checks.yml @@ -11,7 +11,7 @@ env: jobs: fmt: - name: Rustfmt + name: Formatting Check runs-on: ubuntu-latest steps: - name: Checkout repository @@ -44,3 +44,23 @@ jobs: - name: Run Clippy run: cargo clippy --all-targets --all-features -- -D warnings + + test: + name: Test + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Install system dependencies + run: | + sudo apt-get update + sudo apt-get install -y pkg-config libxkbcommon-dev libfontconfig1-dev libfreetype6-dev libglvnd-dev libinput-dev libvulkan-dev libwayland-dev libx11-dev libxcursor-dev libxi-dev libxrandr-dev libasound2-dev libdbus-1-dev + + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + with: + components: clippy + + - name: Run Tests + run: cargo test --all-targets --all-features From 724415b4505b80194882bdc5846675f6875252ad Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Sat, 11 Apr 2026 09:06:39 +0200 Subject: [PATCH 10/10] Review fix: revert format step name change --- .github/workflows/rust-checks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust-checks.yml b/.github/workflows/rust-checks.yml index f9b27b0..ebcdd80 100644 --- a/.github/workflows/rust-checks.yml +++ b/.github/workflows/rust-checks.yml @@ -11,7 +11,7 @@ env: jobs: fmt: - name: Formatting Check + name: Rustfmt runs-on: ubuntu-latest steps: - name: Checkout repository