diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9f12bc7f..f833285a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,7 +19,7 @@ To build Zellij, we're using cargo-make – you can install it by running `cargo Here are some of the commands currently supported by the build system: ```sh -# Format code, build, then run tests +# Format code, build, then run tests and clippy cargo make # You can also perform these actions individually cargo make format diff --git a/Makefile.toml b/Makefile.toml index 8cd80c77..a4388ebc 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -3,6 +3,18 @@ CARGO_MAKE_EXTEND_WORKSPACE_MAKEFILE = true SKIP_TEST = false +# Add clippy to the default flow +[tasks.dev-test-flow] +dependencies = [ + "format-flow", + "format-toml-conditioned-flow", + "pre-build", + "build", + "post-build", + "test-flow", + "clippy", +] + # Patching the default flows to skip testing of wasm32-wasi targets [tasks.pre-test] condition = { env = { "CARGO_MAKE_CRATE_TARGET_TRIPLE" = "wasm32-wasi" } } diff --git a/src/client/pane_resizer.rs b/src/client/pane_resizer.rs index 007dc33f..16eda5c8 100644 --- a/src/client/pane_resizer.rs +++ b/src/client/pane_resizer.rs @@ -1,7 +1,10 @@ use crate::os_input_output::OsApi; use crate::panes::{PaneId, PositionAndSize}; use crate::tab::Pane; -use std::collections::{BTreeMap, HashSet}; +use std::{ + cmp::Ordering, + collections::{BTreeMap, HashSet}, +}; pub struct PaneResizer<'a> { panes: &'a mut BTreeMap>, @@ -29,63 +32,72 @@ impl<'a> PaneResizer<'a> { let mut successfully_resized = false; let mut column_difference: isize = 0; let mut row_difference: isize = 0; - if new_size.columns < current_size.columns { - let reduce_by = current_size.columns - new_size.columns; - find_reducible_vertical_chain( - &self.panes, - reduce_by, - current_size.columns, - current_size.rows, - ) - .map(|panes_to_resize| { - self.reduce_panes_left_and_pull_adjacents_left(panes_to_resize, reduce_by); - column_difference = new_size.columns as isize - current_size.columns as isize; - current_size.columns = (current_size.columns as isize + column_difference) as usize; - successfully_resized = true; - }); - } else if new_size.columns > current_size.columns { - let increase_by = new_size.columns - current_size.columns; - find_increasable_vertical_chain( - &self.panes, - increase_by, - current_size.columns, - current_size.rows, - ) - .map(|panes_to_resize| { - self.increase_panes_right_and_push_adjacents_right(panes_to_resize, increase_by); - column_difference = new_size.columns as isize - current_size.columns as isize; - current_size.columns = (current_size.columns as isize + column_difference) as usize; - successfully_resized = true; - }); + match new_size.columns.cmp(¤t_size.columns) { + Ordering::Greater => { + let increase_by = new_size.columns - current_size.columns; + if let Some(panes_to_resize) = find_increasable_vertical_chain( + &self.panes, + increase_by, + current_size.columns, + current_size.rows, + ) { + self.increase_panes_right_and_push_adjacents_right( + panes_to_resize, + increase_by, + ); + column_difference = new_size.columns as isize - current_size.columns as isize; + current_size.columns = + (current_size.columns as isize + column_difference) as usize; + successfully_resized = true; + }; + } + Ordering::Less => { + let reduce_by = current_size.columns - new_size.columns; + if let Some(panes_to_resize) = find_reducible_vertical_chain( + &self.panes, + reduce_by, + current_size.columns, + current_size.rows, + ) { + self.reduce_panes_left_and_pull_adjacents_left(panes_to_resize, reduce_by); + column_difference = new_size.columns as isize - current_size.columns as isize; + current_size.columns = + (current_size.columns as isize + column_difference) as usize; + successfully_resized = true; + }; + } + Ordering::Equal => (), } - if new_size.rows < current_size.rows { - let reduce_by = current_size.rows - new_size.rows; - find_reducible_horizontal_chain( - &self.panes, - reduce_by, - current_size.columns, - current_size.rows, - ) - .map(|panes_to_resize| { - self.reduce_panes_up_and_pull_adjacents_up(panes_to_resize, reduce_by); - row_difference = new_size.rows as isize - current_size.rows as isize; - current_size.rows = (current_size.rows as isize + row_difference) as usize; - successfully_resized = true; - }); - } else if new_size.rows > current_size.rows { - let increase_by = new_size.rows - current_size.rows; - find_increasable_horizontal_chain( - &self.panes, - increase_by, - current_size.columns, - current_size.rows, - ) - .map(|panes_to_resize| { - self.increase_panes_down_and_push_down_adjacents(panes_to_resize, increase_by); - row_difference = new_size.rows as isize - current_size.rows as isize; - current_size.rows = (current_size.rows as isize + row_difference) as usize; - successfully_resized = true; - }); + match new_size.rows.cmp(¤t_size.rows) { + Ordering::Greater => { + let increase_by = new_size.rows - current_size.rows; + if let Some(panes_to_resize) = find_increasable_horizontal_chain( + &self.panes, + increase_by, + current_size.columns, + current_size.rows, + ) { + self.increase_panes_down_and_push_down_adjacents(panes_to_resize, increase_by); + row_difference = new_size.rows as isize - current_size.rows as isize; + current_size.rows = (current_size.rows as isize + row_difference) as usize; + successfully_resized = true; + }; + } + Ordering::Less => { + let reduce_by = current_size.rows - new_size.rows; + if let Some(panes_to_resize) = find_reducible_horizontal_chain( + &self.panes, + reduce_by, + current_size.columns, + current_size.rows, + ) { + self.reduce_panes_up_and_pull_adjacents_up(panes_to_resize, reduce_by); + row_difference = new_size.rows as isize - current_size.rows as isize; + current_size.rows = (current_size.rows as isize + row_difference) as usize; + successfully_resized = true; + }; + } + Ordering::Equal => (), } if successfully_resized { Some((column_difference, row_difference)) @@ -229,13 +241,12 @@ impl<'a> PaneResizer<'a> { fn find_next_increasable_horizontal_pane( panes: &BTreeMap>, - right_of: &Box, + right_of: &dyn Pane, increase_by: usize, ) -> Option { let next_pane_candidates = panes.values().filter( |p| { - p.x() == right_of.x() + right_of.columns() + 1 - && p.horizontally_overlaps_with(right_of.as_ref()) + p.x() == right_of.x() + right_of.columns() + 1 && p.horizontally_overlaps_with(right_of) }, // TODO: the name here is wrong, it should be vertically_overlaps_with ); let resizable_candidates = @@ -255,11 +266,11 @@ fn find_next_increasable_horizontal_pane( fn find_next_increasable_vertical_pane( panes: &BTreeMap>, - below: &Box, + below: &dyn Pane, increase_by: usize, ) -> Option { let next_pane_candidates = panes.values().filter( - |p| p.y() == below.y() + below.rows() + 1 && p.vertically_overlaps_with(below.as_ref()), // TODO: the name here is wrong, it should be horizontally_overlaps_with + |p| p.y() == below.y() + below.rows() + 1 && p.vertically_overlaps_with(below), // TODO: the name here is wrong, it should be horizontally_overlaps_with ); let resizable_candidates = next_pane_candidates.filter(|p| p.can_increase_width_by(increase_by)); @@ -278,11 +289,11 @@ fn find_next_increasable_vertical_pane( fn find_next_reducible_vertical_pane( panes: &BTreeMap>, - below: &Box, + below: &dyn Pane, reduce_by: usize, ) -> Option { let next_pane_candidates = panes.values().filter( - |p| p.y() == below.y() + below.rows() + 1 && p.vertically_overlaps_with(below.as_ref()), // TODO: the name here is wrong, it should be horizontally_overlaps_with + |p| p.y() == below.y() + below.rows() + 1 && p.vertically_overlaps_with(below), // TODO: the name here is wrong, it should be horizontally_overlaps_with ); let resizable_candidates = next_pane_candidates.filter(|p| p.can_reduce_width_by(reduce_by)); resizable_candidates.fold(None, |next_pane_id, p| match next_pane_id { @@ -300,13 +311,12 @@ fn find_next_reducible_vertical_pane( fn find_next_reducible_horizontal_pane( panes: &BTreeMap>, - right_of: &Box, + right_of: &dyn Pane, reduce_by: usize, ) -> Option { let next_pane_candidates = panes.values().filter( |p| { - p.x() == right_of.x() + right_of.columns() + 1 - && p.horizontally_overlaps_with(right_of.as_ref()) + p.x() == right_of.x() + right_of.columns() + 1 && p.horizontally_overlaps_with(right_of) }, // TODO: the name here is wrong, it should be vertically_overlaps_with ); let resizable_candidates = next_pane_candidates.filter(|p| p.can_reduce_height_by(reduce_by)); @@ -351,7 +361,11 @@ fn find_increasable_horizontal_chain( if current_pane.x() + current_pane.columns() == screen_width { return Some(panes_to_resize); } - match find_next_increasable_horizontal_pane(panes, ¤t_pane, increase_by) { + match find_next_increasable_horizontal_pane( + panes, + current_pane.as_ref(), + increase_by, + ) { Some(next_pane_id) => { current_pane = panes.get(&next_pane_id).unwrap(); } @@ -397,7 +411,11 @@ fn find_increasable_vertical_chain( if current_pane.y() + current_pane.rows() == screen_height { return Some(panes_to_resize); } - match find_next_increasable_vertical_pane(panes, ¤t_pane, increase_by) { + match find_next_increasable_vertical_pane( + panes, + current_pane.as_ref(), + increase_by, + ) { Some(next_pane_id) => { current_pane = panes.get(&next_pane_id).unwrap(); } @@ -443,7 +461,11 @@ fn find_reducible_horizontal_chain( if current_pane.x() + current_pane.columns() == screen_width { return Some(panes_to_resize); } - match find_next_reducible_horizontal_pane(panes, ¤t_pane, reduce_by) { + match find_next_reducible_horizontal_pane( + panes, + current_pane.as_ref(), + reduce_by, + ) { Some(next_pane_id) => { current_pane = panes.get(&next_pane_id).unwrap(); } @@ -489,7 +511,11 @@ fn find_reducible_vertical_chain( if current_pane.y() + current_pane.rows() == screen_height { return Some(panes_to_resize); } - match find_next_reducible_vertical_pane(panes, ¤t_pane, increase_by) { + match find_next_reducible_vertical_pane( + panes, + current_pane.as_ref(), + increase_by, + ) { Some(next_pane_id) => { current_pane = panes.get(&next_pane_id).unwrap(); } diff --git a/src/client/panes/grid.rs b/src/client/panes/grid.rs index 7651bdc0..87df3023 100644 --- a/src/client/panes/grid.rs +++ b/src/client/panes/grid.rs @@ -29,11 +29,9 @@ fn get_top_non_canonical_rows(rows: &mut Vec) -> Vec { fn get_bottom_canonical_row_and_wraps(rows: &mut Vec) -> Vec { let mut index_of_last_non_canonical_row = None; for (i, row) in rows.iter().enumerate().rev() { + index_of_last_non_canonical_row = Some(i); if row.is_canonical { - index_of_last_non_canonical_row = Some(i); break; - } else { - index_of_last_non_canonical_row = Some(i); } } match index_of_last_non_canonical_row { @@ -496,9 +494,7 @@ impl Grid { } pub fn add_character(&mut self, terminal_character: TerminalCharacter) { // TODO: try to separate adding characters from moving the cursors in this function - if self.cursor.x < self.width { - self.insert_character_at_cursor_position(terminal_character); - } else { + if self.cursor.x >= self.width { // line wrap self.cursor.x = 0; if self.cursor.y == self.height - 1 { @@ -519,8 +515,8 @@ impl Grid { self.viewport.push(line_wrapped_row); } } - self.insert_character_at_cursor_position(terminal_character); } + self.insert_character_at_cursor_position(terminal_character); self.move_cursor_forward_until_edge(1); } pub fn move_cursor_forward_until_edge(&mut self, count: usize) { diff --git a/src/client/panes/plugin_pane.rs b/src/client/panes/plugin_pane.rs index 49cf2aee..24daec08 100644 --- a/src/client/panes/plugin_pane.rs +++ b/src/client/panes/plugin_pane.rs @@ -1,5 +1,3 @@ -#![allow(clippy::clippy::if_same_then_else)] - use crate::{common::SenderWithContext, pty_bus::VteBytes, tab::Pane, wasm_vm::PluginInstruction}; use std::{sync::mpsc::channel, unimplemented}; diff --git a/src/client/panes/terminal_character.rs b/src/client/panes/terminal_character.rs index ce967ef8..7569e993 100644 --- a/src/client/panes/terminal_character.rs +++ b/src/client/panes/terminal_character.rs @@ -48,7 +48,7 @@ pub enum NamedColor { } impl NamedColor { - fn to_foreground_ansi_code(&self) -> String { + fn to_foreground_ansi_code(self) -> String { match self { NamedColor::Black => format!("{}", 30), NamedColor::Red => format!("{}", 31), @@ -68,7 +68,7 @@ impl NamedColor { NamedColor::BrightWhite => format!("{}", 97), } } - fn to_background_ansi_code(&self) -> String { + fn to_background_ansi_code(self) -> String { match self { NamedColor::Black => format!("{}", 40), NamedColor::Red => format!("{}", 41), diff --git a/src/client/panes/terminal_pane.rs b/src/client/panes/terminal_pane.rs index d2473ba3..fd9bbb07 100644 --- a/src/client/panes/terminal_pane.rs +++ b/src/client/panes/terminal_pane.rs @@ -1,5 +1,3 @@ -#![allow(clippy::clippy::if_same_then_else)] - use crate::tab::Pane; use ::nix::pty::Winsize; use ::std::os::unix::io::RawFd; @@ -566,12 +564,6 @@ impl vte::Perform for TerminalPane { } else { self.grid.clear_scroll_region(); } - } else if c == 't' { - // TBD - title? - } else if c == 'n' { - // TBD - device status report - } else if c == 'c' { - // TBD - identify terminal } else if c == 'M' { // delete lines if currently inside scroll region let line_count_to_delete = if params[0] == 0 { diff --git a/src/client/tab.rs b/src/client/tab.rs index 9cafdcec..07e0f042 100644 --- a/src/client/tab.rs +++ b/src/client/tab.rs @@ -205,6 +205,7 @@ pub trait Pane { impl Tab { // FIXME: Too many arguments here! Maybe bundle all of the senders for the whole program in a struct? + #[allow(clippy::too_many_arguments)] pub fn new( index: usize, position: usize, @@ -1720,24 +1721,22 @@ impl Tab { // this is not ideal but until we get rid of expansion_boundary, it's a necessity self.toggle_active_pane_fullscreen(); } - match PaneResizer::new(&mut self.panes, &mut self.os_api) - .resize(self.full_screen_ws, new_screen_size) + if let Some((column_difference, row_difference)) = + PaneResizer::new(&mut self.panes, &mut self.os_api) + .resize(self.full_screen_ws, new_screen_size) { - Some((column_difference, row_difference)) => { - self.should_clear_display_before_rendering = true; - self.expansion_boundary.as_mut().map(|expansion_boundary| { - // TODO: this is not always accurate - expansion_boundary.columns = - (expansion_boundary.columns as isize + column_difference) as usize; - expansion_boundary.rows = - (expansion_boundary.rows as isize + row_difference) as usize; - }); - self.full_screen_ws.columns = - (self.full_screen_ws.columns as isize + column_difference) as usize; - self.full_screen_ws.rows = - (self.full_screen_ws.rows as isize + row_difference) as usize; - } - None => {} + self.should_clear_display_before_rendering = true; + if let Some(expansion_boundary) = self.expansion_boundary.as_mut() { + // TODO: this is not always accurate + expansion_boundary.columns = + (expansion_boundary.columns as isize + column_difference) as usize; + expansion_boundary.rows = + (expansion_boundary.rows as isize + row_difference) as usize; + }; + self.full_screen_ws.columns = + (self.full_screen_ws.columns as isize + column_difference) as usize; + self.full_screen_ws.rows = + (self.full_screen_ws.rows as isize + row_difference) as usize; }; } pub fn resize_left(&mut self) { diff --git a/src/common/input/config.rs b/src/common/input/config.rs index 4bb9e10f..1926b11c 100644 --- a/src/common/input/config.rs +++ b/src/common/input/config.rs @@ -51,7 +51,6 @@ impl Config { } /// Deserializes from given path. - #[allow(unused_must_use)] pub fn new(path: &Path) -> ConfigResult { match File::open(path) { Ok(mut file) => { @@ -90,7 +89,6 @@ impl Config { } } - //#[allow(unused_must_use)] /// In order not to mess up tests from changing configurations #[cfg(test)] pub fn from_cli_config(_: Option) -> ConfigResult { diff --git a/src/common/os_input_output.rs b/src/common/os_input_output.rs index 013b9f12..35d77fdd 100644 --- a/src/common/os_input_output.rs +++ b/src/common/os_input_output.rs @@ -84,15 +84,10 @@ fn handle_command_exit(mut child: Child) { } for signal in signals.pending() { - // FIXME: We need to handle more signals here! - #[allow(clippy::single_match)] - match signal { - SIGINT => { - child.kill().unwrap(); - child.wait().unwrap(); - break 'handle_exit; - } - _ => {} + if let SIGINT = signal { + child.kill().unwrap(); + child.wait().unwrap(); + break 'handle_exit; } } } diff --git a/src/common/screen.rs b/src/common/screen.rs index 1e37ca4f..01ac04fc 100644 --- a/src/common/screen.rs +++ b/src/common/screen.rs @@ -81,7 +81,9 @@ pub struct Screen { } impl Screen { + // FIXME: This lint needs actual fixing! Maybe by bundling the Senders /// Creates and returns a new [`Screen`]. + #[allow(clippy::too_many_arguments)] pub fn new( receive_screen_instructions: Receiver<(ScreenInstruction, ErrorContext)>, send_pty_instructions: SenderWithContext, diff --git a/src/common/utils/logging.rs b/src/common/utils/logging.rs index d08639aa..06827bfc 100644 --- a/src/common/utils/logging.rs +++ b/src/common/utils/logging.rs @@ -47,8 +47,7 @@ pub fn _debug_log_to_file_pid_3(message: String, pid: RawFd) -> io::Result<()> { } } -#[allow(dead_code)] -pub fn delete_log_file() -> io::Result<()> { +pub fn _delete_log_file() -> io::Result<()> { if fs::metadata(ZELLIJ_TMP_LOG_FILE).is_ok() { fs::remove_file(ZELLIJ_TMP_LOG_FILE) } else { @@ -56,8 +55,7 @@ pub fn delete_log_file() -> io::Result<()> { } } -#[allow(dead_code)] -pub fn delete_log_dir() -> io::Result<()> { +pub fn _delete_log_dir() -> io::Result<()> { if fs::metadata(ZELLIJ_TMP_LOG_DIR).is_ok() { fs::remove_dir_all(ZELLIJ_TMP_LOG_DIR) } else { diff --git a/src/common/utils/shared.rs b/src/common/utils/shared.rs index 93892d4f..d5ba943b 100644 --- a/src/common/utils/shared.rs +++ b/src/common/utils/shared.rs @@ -18,9 +18,9 @@ pub fn adjust_to_size(s: &str, rows: usize, columns: usize) -> String { if actual_len > columns { let mut line = String::from(l); line.truncate(columns); - return line; + line } else { - return [l, &str::repeat(" ", columns - ansi_len(l))].concat(); + [l, &str::repeat(" ", columns - ansi_len(l))].concat() } }) .chain(iter::repeat(str::repeat(" ", columns)))