From f7c41aef0b5ae6fe3a95e3d434297fbe7d64fffa Mon Sep 17 00:00:00 2001 From: Maksym Date: Tue, 19 Aug 2025 21:49:03 +0200 Subject: [PATCH 1/4] refactor: remove unused code, fix clippy warnings --- src/app.rs | 56 +++++++++++++++++++-------------------------- src/config.rs | 1 - src/key_shortcut.rs | 1 - src/operations.rs | 2 -- src/pike.rs | 11 ++------- src/ui.rs | 20 +--------------- 6 files changed, 27 insertions(+), 64 deletions(-) diff --git a/src/app.rs b/src/app.rs index 3c33b50..d6c9665 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,6 +1,6 @@ use std::{ env, - io::{self, ErrorKind}, + io::{self}, path::PathBuf, process, rc::Rc, @@ -27,14 +27,12 @@ use crate::{ }; /// TUI application which displays the UI and handles events -#[allow(dead_code)] pub struct App { exit: bool, backend: Pike, ui_state: UIState, } -#[allow(dead_code, unused_variables, unused_mut)] impl App { pub fn build(args: Args) -> App { let cwd = env::current_dir().map_err(|_| "Failed to get current working directory"); @@ -45,7 +43,6 @@ impl App { let config_path = args.config.map(PathBuf::from); let file_path = args.file.map(PathBuf::from); - let no_file_open = file_path.is_none(); let backend: Result = Pike::build(cwd.expect("Error case was handled"), file_path, config_path); @@ -78,6 +75,7 @@ impl App { } /// Builds an app with the default configuration and no open file + #[cfg(test)] fn build_default() -> Self { App::build(Args { config: None, @@ -104,7 +102,7 @@ impl App { let cursor_pos = self.backend.cursor_position(); - let mut render_cursor_position; + let render_cursor_position; self.render_buffer_contents(main_area, frame.buffer_mut()); @@ -408,7 +406,11 @@ impl App { return Ok(()); } - if !key.modifiers.contains(KeyModifiers::CONTROL) && self.try_handle_input_key(key)? { + if self.try_write_char(key)? { + return Ok(()); + } + + if self.try_handle_special_keys(key)? { return Ok(()); } @@ -470,28 +472,37 @@ impl App { } } - fn try_handle_input_key(&mut self, key: KeyEvent) -> Result { - if self.backend.current_buffer().is_none() { + fn try_write_char(&mut self, key: KeyEvent) -> Result { + if self.backend.current_buffer().is_none() || key.modifiers.contains(KeyModifiers::CONTROL) + { return Ok(false); } if let KeyCode::Char(ch) = key.code { self.backend .write_to_current_buffer(&ch.to_string()) - .map_err(|e| io::Error::new(ErrorKind::Other, e.to_string()))?; + .map_err(|e| io::Error::other(e.to_string()))?; - return Ok(true); + Ok(true) + } else { + Ok(false) + } + } + + fn try_handle_special_keys(&mut self, key: KeyEvent) -> Result { + if self.backend.current_buffer().is_none() { + return Ok(false); } match key.code { KeyCode::Enter => { self.backend .write_to_current_buffer("\n") - .map_err(|e| io::Error::new(ErrorKind::Other, e.to_string()))?; + .map_err(|e| io::Error::other(e.to_string()))?; Ok(true) } KeyCode::Tab => { self.backend .write_to_current_buffer(" ") - .map_err(|e| io::Error::new(ErrorKind::Other, e.to_string()))?; + .map_err(|e| io::Error::other(e.to_string()))?; Ok(true) } KeyCode::Backspace => { @@ -519,7 +530,7 @@ impl App { } fn handle_save_operation(&mut self) { - if let Some(path) = self.backend.current_buffer_path() { + if let Some(_path) = self.backend.current_buffer_path() { if let Err(err) = self.backend.save_current_buffer() { eprintln!("Failed to save buffer: {}", err); } @@ -707,25 +718,6 @@ mod tests { assert_eq!(buf, expected) } - #[allow(dead_code)] - /// Helper function to assert the position to render the cursor at in the visible - /// buffer after syncing the buffer contents and cursor position from the backend. - fn assert_cursor_render_pos_no_input(app: &mut App, buf: &Buffer, expected: (u16, u16)) { - let cursor_position = app.backend.cursor_position(); - - if let Some(cp) = cursor_position { - app.ui_state - .buffer_state - .update_x_offset(buf.area, cp.offset); - app.ui_state.buffer_state.update_y_offset(buf.area, cp.line); - } - - let pos = app - .ui_state - .calculate_cursor_for_buffer(buf.area, cursor_position); - - assert_eq!(pos, expected.into()); - } /// The cursor should not move past the bounds of the buffer #[test] fn test_cant_move_cursor_too_far_right() { diff --git a/src/config.rs b/src/config.rs index b9a7766..d3384f0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -34,7 +34,6 @@ pub struct Config { pub key_mappings: HashMap, } -#[allow(dead_code)] impl Config { /// Creates a config instance based on toml string representation pub fn from_toml_representation(s: &str) -> Result { diff --git a/src/key_shortcut.rs b/src/key_shortcut.rs index 4de2079..178522b 100644 --- a/src/key_shortcut.rs +++ b/src/key_shortcut.rs @@ -13,7 +13,6 @@ impl From for KeyShortcut { } } -#[allow(dead_code)] impl KeyShortcut { pub fn new(code: KeyCode, modifiers: KeyModifiers) -> KeyShortcut { KeyShortcut { code, modifiers } diff --git a/src/operations.rs b/src/operations.rs index f2d7562..340a2db 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -1,4 +1,3 @@ -#[allow(dead_code, unused_variables, unused_mut)] /// Every keymappable operation within pike #[derive(Debug, PartialEq, Eq, Clone, Hash)] pub enum Operation { @@ -13,7 +12,6 @@ pub enum Operation { Quit, } -#[allow(dead_code, unused_variables, unused_mut)] impl Operation { /// Creates a new Operation from a string from a config file pub fn from_string(query: &str) -> Result { diff --git a/src/pike.rs b/src/pike.rs index 7efa0d0..46f8b32 100644 --- a/src/pike.rs +++ b/src/pike.rs @@ -33,14 +33,12 @@ pub struct Highlight { } /// Backend of the app -#[allow(dead_code, unused_variables, unused_mut)] pub struct Pike { workspace: Workspace, config: Config, cursor_history: CursorHistory, } -#[allow(dead_code, unused_variables, unused_mut)] impl Pike { /// Create a new instance of Pike in a given directory pub fn build( @@ -171,8 +169,6 @@ impl Pike { let lines: Vec<&str> = data.split('\n').collect(); - let current_line_length = lines.get(pos.line).map_or(0, |line| line.len()); - if pos.offset == 0 && pos.line > 0 { buffer.cursor.move_up(); @@ -225,6 +221,7 @@ impl Pike { /// Returns whether the current buffer has unsaved changes or /// false if it's empty + #[cfg(test)] pub fn has_unsaved_changes(&self) -> bool { match &self.current_buffer() { Some(buffer) => buffer.modified(), @@ -399,6 +396,7 @@ impl Pike { } /// Returns the length of the current line + #[cfg(test)] pub fn current_line_length(&self) -> usize { let current_line_number = self.cursor_position().map_or(0, |pos| pos.line); match self @@ -505,11 +503,6 @@ impl Pike { } } - /// Returns the current working directory as a pathbuf - fn cwd(&self) -> PathBuf { - self.workspace.path.clone() - } - /// Gets an operation corresponding to a key shortcut pub fn get_keymap(&self, mapping: &KeyShortcut) -> Option<&Operation> { self.config.key_mappings.get(mapping) diff --git a/src/ui.rs b/src/ui.rs index 279c8f5..f1a4c1b 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -12,16 +12,6 @@ use tui_input::{Input, InputRequest}; use crate::pike::Highlight; -/// We would like to have some struct which can be rendered -/// as a list with given callbacks to be executed when something is -/// selected (similarly to telescope.nvim) so that we can reuse -/// it when searching for files or word occurrences in the cwd -/// As of now, I can't look that far into the future without writing -/// some code to know what fields this should keep and how it should -/// behave, so it's empty -#[allow(dead_code)] -struct Picker {} - const HIGHLIGHT_BG_SELECTED: Color = Color::Rgb(245, 206, 88); const HIGHLIGHT_BG_UNSELECTED: Color = Color::Rgb(240, 137, 48); @@ -66,7 +56,6 @@ impl From<(&str, FileInputRole)> for FileInputState { /// Holds the information about the current state of the UI /// of the app. -#[allow(dead_code)] #[derive(Default)] pub struct UIState { /// Offset of the currently rendered buffer @@ -206,7 +195,6 @@ impl UIState { /// displayed from line 6 until either the end of the buffer -> /// BufferDisplayOffset{ 0, 6 }. Used to consistently shift the buffer /// when rendering. Persisted in UIState between renders. -#[allow(dead_code)] #[derive(Default)] pub struct BufferDisplayOffset { /// X offset of the line pointed at by the cursor @@ -215,12 +203,7 @@ pub struct BufferDisplayOffset { pub y: usize, } -#[allow(dead_code)] -impl BufferDisplayOffset { - pub fn new(x: usize, y: usize) -> Self { - BufferDisplayOffset { x, y } - } -} +impl BufferDisplayOffset {} #[derive(Default)] pub struct HighlightState { pub highlights: Vec, @@ -234,7 +217,6 @@ pub struct BufferDisplayState { pub highlight_state: HighlightState, } -#[allow(dead_code)] impl BufferDisplayState { pub fn new(offset: BufferDisplayOffset) -> Self { BufferDisplayState { From 786aa392a9195a32839fddd9e380d936733dd754 Mon Sep 17 00:00:00 2001 From: Maksym Date: Tue, 19 Aug 2025 23:04:53 +0200 Subject: [PATCH 2/4] refactor: further fixes for pedantic clippy --- src/app.rs | 73 ++++++++++++++++++++++++------------------------------ src/ui.rs | 39 +++++++++++++++-------------- 2 files changed, 53 insertions(+), 59 deletions(-) diff --git a/src/app.rs b/src/app.rs index d6c9665..088fdf6 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,7 +1,7 @@ use std::{ env, io::{self}, - path::PathBuf, + path::{Path, PathBuf}, process, rc::Rc, }; @@ -50,7 +50,7 @@ impl App { match backend { Ok(backend) => App::new(backend), Err(err) => { - eprintln!("{}", err); + eprintln!("{err}"); process::exit(1); } } @@ -112,27 +112,27 @@ impl App { if let Some(ref input_state) = file_input_value { self.render_file_input(status_bar_area, frame.buffer_mut()); render_cursor_position = self.ui_state.calculate_cursor_position( - CursorCalculationMode::FileInput(&input_state.input), + &CursorCalculationMode::FileInput(&input_state.input), &layout, cursor_pos, ); } else if let Some(ref search_input) = search_input_value { self.render_search_input(status_bar_area, frame.buffer_mut()); render_cursor_position = self.ui_state.calculate_cursor_position( - CursorCalculationMode::FileInput(search_input), + &CursorCalculationMode::FileInput(search_input), &layout, cursor_pos, ); } else { render_cursor_position = self.ui_state.calculate_cursor_position( - CursorCalculationMode::Buffer, + &CursorCalculationMode::Buffer, &layout, cursor_pos, ); self.render_status_bar(status_bar_area, frame.buffer_mut()); } - self.render_cursor(frame, render_cursor_position); + Self::render_cursor(frame, render_cursor_position); } /// Splits an area using the main app layout and returns the @@ -159,7 +159,7 @@ impl App { fn render_buffer_contents(&mut self, area: Rect, buf: &mut ratatui::prelude::Buffer) { // Display a welcome message if no buffer is open if self.backend.current_buffer().is_none() { - self.render_welcome_banner(area, buf); + Self::render_welcome_banner(area, buf); } let contents = self.backend.current_buffer_contents(); @@ -169,7 +169,7 @@ impl App { widget.render(area, buf, &mut self.ui_state.buffer_state); } - fn render_welcome_banner(&self, area: Rect, buf: &mut ratatui::prelude::Buffer) { + fn render_welcome_banner(area: Rect, buf: &mut ratatui::prelude::Buffer) { let banner = WELCOME_MESSAGE; let paragraph = Paragraph::new(banner).block(Block::default().borders(Borders::NONE)); paragraph.render(area, buf); @@ -181,7 +181,7 @@ impl App { let is_modified = self.backend.is_current_buffer_modified(); let indicator = if is_modified { "*" } else { "" }; - let text_widget = Text::from(format!("{}{}", filename, indicator)); + let text_widget = Text::from(format!("{filename}{indicator}")); let paragraph_widget = Paragraph::new(text_widget).wrap(Wrap { trim: false }); let block_widget = paragraph_widget.block(Block::default().borders(Borders::TOP)); @@ -190,7 +190,7 @@ impl App { } /// Render the cursor in a given position - fn render_cursor(&self, frame: &mut ratatui::prelude::Frame, position: TerminalPosition) { + fn render_cursor(frame: &mut ratatui::prelude::Frame, position: TerminalPosition) { frame.set_cursor_position(position); } @@ -226,7 +226,7 @@ impl App { self.ui_state.search_input = None; } - /// Open a file input with the given contents and store it in UIState + /// Open a file input with the given contents and store it in `UIState` fn open_file_input(&mut self, contents: &str, role: FileInputRole) { self.ui_state.file_input = Some((contents, role).into()); } @@ -254,16 +254,15 @@ impl App { /// indicating whether the event has been handled or not. fn try_handle_key_press_with_file_input(&mut self, key: KeyEvent) -> bool { // No input means the event can't be handled - let input = match self.ui_state.file_input.as_mut() { - Some(input) => input, - None => return false, + let Some(input) = self.ui_state.file_input.as_mut() else { + return false; }; // Perform the corresponding operation and close the input if (key.code, key.modifiers) == (KeyCode::Enter, KeyModifiers::NONE) { let path = input.to_path(); match input.role { - FileInputRole::GetOpenPath => self.open_file_from_path(path), + FileInputRole::GetOpenPath => self.open_file_from_path(&path), FileInputRole::GetSavePath => { self.backend.bind_current_buffer_to_path(path); self.handle_save_operation(); @@ -295,9 +294,8 @@ impl App { /// Returns a boolean indicating whether the event has been handled or not. fn try_handle_key_press_with_search_input(&mut self, key: KeyEvent) -> bool { // No input means the event can't be handled - let input = match self.ui_state.search_input.as_mut() { - Some(input) => input, - None => return false, + let Some(input) = self.ui_state.search_input.as_mut() else { + return false; }; // Perform the corresponding operation and close the input @@ -307,7 +305,7 @@ impl App { .backend .search_in_current_buffer(&query) .unwrap_or_else(|err| { - eprintln!("Error searching in buffer: {}", err); + eprintln!("Error searching in buffer: {err}"); vec![] }); @@ -366,21 +364,18 @@ impl App { } /// Open a file from a given path - fn open_file_from_path(&mut self, path: PathBuf) { + fn open_file_from_path(&mut self, path: &Path) { self.backend - .create_and_open_file(&path) + .create_and_open_file(path) // TODO: display message in the UI .expect("Error opening file!"); } - /// Try to convert a given key event to an InputRequest to be sent to a tui_input::Input + /// Try to convert a given key event to an `InputRequest` to be sent to a `tui_input::Input` /// instance. fn key_event_to_input_request(key: KeyEvent) -> Option { match (key.code, key.modifiers) { - (KeyCode::Char(chr), KeyModifiers::NONE) => { - Some(tui_input::InputRequest::InsertChar(chr)) - } - (KeyCode::Char(chr), KeyModifiers::SHIFT) => { + (KeyCode::Char(chr), KeyModifiers::SHIFT | KeyModifiers::NONE) => { Some(tui_input::InputRequest::InsertChar(chr)) } (KeyCode::Backspace, KeyModifiers::NONE) => { @@ -532,7 +527,7 @@ impl App { fn handle_save_operation(&mut self) { if let Some(_path) = self.backend.current_buffer_path() { if let Err(err) = self.backend.save_current_buffer() { - eprintln!("Failed to save buffer: {}", err); + eprintln!("Failed to save buffer: {err}"); } } else { // Ask for filepath if the buffer is not bound to one @@ -569,7 +564,7 @@ mod tests { temp_file_with_contents, ui::{n_spaces, solid_border}, }, - ui::FileInputRole, + ui::{self, FileInputRole}, }; use super::App; @@ -602,6 +597,7 @@ mod tests { /// Used in unit tests to provide the UI element, based on which the cursor /// position should be calculated, so that a testing buffer can be created only /// to accommodate this element instead of the whole UI. + #[derive(Copy, Clone)] enum CursorRenderingWidget { CurrentBuffer, FileInput, @@ -639,15 +635,14 @@ mod tests { .file_input .as_ref() .expect("A file input should be open when testing cursor in file input"); - app.ui_state - .calculate_cursor_for_file_input(&input.input, buf.area) + ui::UIState::calculate_cursor_for_file_input(&input.input, buf.area) } }; assert_eq!(pos, expected.into()); } - /// Shorthand for defining the renderer in unit tests and calling assert_cursor_render_pos + /// Shorthand for defining the renderer in unit tests and calling `assert_cursor_render_pos` fn acrp_based_on_current_buffer( app: &mut App, buf: &ratatui::buffer::Buffer, @@ -715,7 +710,7 @@ mod tests { let mut buf = Buffer::empty(Rect::new(0, 0, width, 2)); let expected = Buffer::with_lines(vec![solid_border(width.into()), filename.to_string()]); app.render_status_bar(buf.area, &mut buf); - assert_eq!(buf, expected) + assert_eq!(buf, expected); } /// The cursor should not move past the bounds of the buffer @@ -878,7 +873,7 @@ mod tests { let buf = Buffer::empty(Rect::new(0, 0, 4, 1)); app.open_file_input("hello, world!", FileInputRole::GetOpenPath); // Does not reach (3, 1) because of the border - acrp_based_on_file_input(&mut app, &buf, (2, 1)) + acrp_based_on_file_input(&mut app, &buf, (2, 1)); } #[test] @@ -907,7 +902,7 @@ mod tests { app.handle_key_event(close_event) .expect("Failed to handle key event"); - assert!(app.exit) + assert!(app.exit); } #[test] @@ -986,8 +981,7 @@ mod tests { for (event, expected_pos) in navigation_cases { assert!( app.try_handle_navigation(event), - "Navigation event {:?} was not handled", - event + "Navigation event {event:?} was not handled", ); acrp_based_on_current_buffer(&mut app, &buf, expected_pos); } @@ -1001,8 +995,7 @@ mod tests { let event = KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE); assert!( !app.try_handle_navigation(event), - "Navigation event {:?} was handled", - event + "Navigation event {event:?} was handled", ); acrp_based_on_current_buffer(&mut app, &buf, (0, 0)); } @@ -1088,7 +1081,7 @@ mod tests { KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE), ]; - for event in wor_query_key_events.iter() { + for event in &wor_query_key_events { app.handle_key_event(*event) .expect("Failed to handle key event"); } @@ -1151,7 +1144,7 @@ mod tests { KeyEvent::new(KeyCode::Char('o'), KeyModifiers::NONE), ]; - for event in hello.iter() { + for event in &hello { app.handle_key_event(*event) .expect("Failed to handle key event"); } diff --git a/src/ui.rs b/src/ui.rs index f1a4c1b..f29eb5d 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -70,7 +70,7 @@ impl UIState { /// Calculate the cursor position for a given `CursorCalculation` mode pub fn calculate_cursor_position( &self, - calc_mode: CursorCalculationMode, + calc_mode: &CursorCalculationMode, layout: &Rc<[Rect]>, cursor_pos: Option, ) -> TerminalPosition { @@ -79,7 +79,7 @@ impl UIState { match calc_mode { CursorCalculationMode::FileInput(input) => { - self.calculate_cursor_for_file_input(input, layout[main_area]) + Self::calculate_cursor_for_file_input(input, layout[main_area]) } CursorCalculationMode::Buffer => { self.calculate_cursor_for_buffer(layout[status_bar_area], cursor_pos) @@ -88,16 +88,16 @@ impl UIState { } /// Calculate position for file input - pub fn calculate_cursor_for_file_input(&self, input: &Input, area: Rect) -> TerminalPosition { + pub fn calculate_cursor_for_file_input(input: &Input, area: Rect) -> TerminalPosition { let border_offset = 1; let max_x = { - let (x, _) = Self::max_rect_position(&area); + let (x, _) = Self::max_rect_position(area); x.saturating_sub(border_offset) }; let (base_x, base_y) = { - let (x, y) = Self::base_rect_position(&area); + let (x, y) = Self::base_rect_position(area); (x + border_offset, y) }; @@ -115,8 +115,8 @@ impl UIState { // If we have a cursor position, compute accordingly; // otherwise return a default if let Some(cursor_pos) = cursor_pos { - let (max_x, max_y) = Self::max_rect_position(&area); - let (base_x, base_y) = Self::base_rect_position(&area); + let (max_x, max_y) = Self::max_rect_position(area); + let (base_x, base_y) = Self::base_rect_position(area); let x_offset = self.buffer_state.offset.x as u16; let y_offset = self.buffer_state.offset.y as u16; @@ -134,12 +134,12 @@ impl UIState { } /// Calculate the maximum renderable position in a given area - fn max_rect_position(area: &Rect) -> (u16, u16) { + fn max_rect_position(area: Rect) -> (u16, u16) { (area.width.saturating_sub(1), area.height.saturating_sub(1)) } /// Calculate the base (top-left) position in a given area - fn base_rect_position(area: &Rect) -> (u16, u16) { + fn base_rect_position(area: Rect) -> (u16, u16) { (area.x, area.y) } @@ -193,8 +193,8 @@ impl UIState { /// Holds the information how much offset is the /// current buffer when displayed - for example, it's /// displayed from line 6 until either the end of the buffer -> -/// BufferDisplayOffset{ 0, 6 }. Used to consistently shift the buffer -/// when rendering. Persisted in UIState between renders. +/// `BufferDisplayOffset`{ 0, 6 }. Used to consistently shift the buffer +/// when rendering. Persisted in `UIState` between renders. #[derive(Default)] pub struct BufferDisplayOffset { /// X offset of the line pointed at by the cursor @@ -253,7 +253,7 @@ impl BufferDisplayState { /// Shifts the content of the buffer down by the offset and returns the resulting string. /// Basically removes the first self.offset.y lines and joins the remaining ones. - fn shift_contents_down(&mut self, contents: String) -> String { + fn shift_contents_down(&mut self, contents: &str) -> String { contents .lines() .skip(self.offset.y) @@ -277,7 +277,7 @@ impl BufferDisplayState { /// Shifts the contents of the buffer down and to the right by the offset and returns the /// resulting string. - fn shift_contents(&mut self, contents: String) -> String { + fn shift_contents(&mut self, contents: &str) -> String { let down_shifted = self.shift_contents_down(contents); self.shift_contents_right(down_shifted) } @@ -335,13 +335,14 @@ impl BufferDisplayState { /// Prepares a paragraph widget with the given contents, applying highlights if present. fn prepare_paragraph_widget<'a>(&mut self, contents: &'a str) -> Paragraph<'a> { - let paragraph_widget = if !self.highlight_state.highlights.is_empty() { - let text_widget = self.add_highlights(contents, &self.highlight_state.highlights); + let paragraph_widget = if self.highlight_state.highlights.is_empty() { + let text_widget = Text::from(contents); Paragraph::new(text_widget) } else { - let text_widget = Text::from(contents); + let text_widget = self.add_highlights(contents, &self.highlight_state.highlights); Paragraph::new(text_widget) }; + paragraph_widget } } @@ -378,7 +379,7 @@ impl StatefulWidget for BufferDisplayWidget<'_> { state.update_y_offset(area, pos.line); } // Shift contents based on offset - let shifted_contents = state.shift_contents(self.buffer_contents.to_string()); + let shifted_contents = state.shift_contents(self.buffer_contents); // Render the text using Paragraph let paragraph_widget = state.prepare_paragraph_widget(&shifted_contents); @@ -402,7 +403,7 @@ impl StatefulWidget for FileInput { .borders(widgets::Borders::all()) .title("Enter relative file path"), ); - widget.render(area, buf) + widget.render(area, buf); } } @@ -418,7 +419,7 @@ impl StatefulWidget for SearchInput { .borders(widgets::Borders::all()) .title("Search for: "), ); - widget.render(area, buf) + widget.render(area, buf); } } From 12ef415094f338625e68d7022ac301ee8729d3e2 Mon Sep 17 00:00:00 2001 From: Maksym Date: Tue, 19 Aug 2025 23:26:55 +0200 Subject: [PATCH 3/4] refactor: fix the rest of pedantic clippy --- src/app.rs | 7 +++++-- src/config.rs | 4 ++-- src/key_shortcut.rs | 14 +++++++------- src/pike.rs | 47 +++++++++++++++++++++++---------------------- src/test_util.rs | 2 +- src/ui.rs | 27 ++++++++++++++------------ src/welcome_pike.rs | 4 ++-- 7 files changed, 56 insertions(+), 49 deletions(-) diff --git a/src/app.rs b/src/app.rs index 088fdf6..d987528 100644 --- a/src/app.rs +++ b/src/app.rs @@ -44,8 +44,11 @@ impl App { let config_path = args.config.map(PathBuf::from); let file_path = args.file.map(PathBuf::from); - let backend: Result = - Pike::build(cwd.expect("Error case was handled"), file_path, config_path); + let backend: Result = Pike::build( + &cwd.expect("Error case was handled"), + file_path, + config_path, + ); match backend { Ok(backend) => App::new(backend), diff --git a/src/config.rs b/src/config.rs index d3384f0..521a6f4 100644 --- a/src/config.rs +++ b/src/config.rs @@ -95,12 +95,12 @@ impl Config { let op = Operation::from_string(op.as_str().unwrap())?; if !seen_shortcuts.insert(shortcut.clone()) { - return Err(format!("Duplicate keybinding found: {:?}", shortcut)); + return Err(format!("Duplicate keybinding found: {shortcut:?}")); } // Check for duplicate operations if !seen_operations.insert(op.clone()) { - return Err(format!("Duplicate keymap operation found: {:?}", op)); + return Err(format!("Duplicate keymap operation found: {op:?}")); } return_value.push((op, shortcut)); diff --git a/src/key_shortcut.rs b/src/key_shortcut.rs index 178522b..66585a8 100644 --- a/src/key_shortcut.rs +++ b/src/key_shortcut.rs @@ -18,11 +18,11 @@ impl KeyShortcut { KeyShortcut { code, modifiers } } - /// Creates a new KeyShortcut based on a string from a config file. - /// String representation of the shortcut follows the VSCode notation, + /// Creates a new `KeyShortcut` based on a string from a config file. + /// String representation of the shortcut follows the `VSCode` notation, /// e.g. ctrl+shift+p, ctrl+alt+del pub fn from_string(s: &str) -> Result { - let elements_lowercase = s.split("+").map(|s| s.to_lowercase()); + let elements_lowercase = s.split('+').map(str::to_lowercase); let mut modifiers = KeyModifiers::empty(); let mut code = KeyCode::Null; @@ -52,7 +52,7 @@ impl KeyShortcut { } } -/// Returns a KeyModifiers object from a string representation +/// Returns a `KeyModifiers` object from a string representation /// or None if it does not match any. fn key_modifier_from_string(s: &str) -> Option { match s { @@ -63,7 +63,7 @@ fn key_modifier_from_string(s: &str) -> Option { } } -/// Returns a KeyCode from a string representation. +/// Returns a `KeyCode` from a string representation. /// The input should be mappable to a valid keycode and not a modifier. fn keycode_from_string(s: &str) -> Result { let return_value = match s { @@ -133,7 +133,7 @@ mod key_shortcut_test { (KeyEvent::new(KeyCode::F(1), KeyModifiers::SHIFT)), ]; - let shortcuts: [KeyShortcut; 7] = events.map(|event| event.into()); + let shortcuts: [KeyShortcut; 7] = events.map(std::convert::Into::into); for (event, shortcut) in events.iter().zip(shortcuts.iter()) { assert_eq!( @@ -151,7 +151,7 @@ mod key_shortcut_test { #[test] fn from_string_valid_test_cases() { - let strings_and_keymaps = vec![ + let strings_and_keymaps = [ ( "q", KeyShortcut::new(KeyCode::Char('q'), KeyModifiers::empty()), diff --git a/src/pike.rs b/src/pike.rs index 46f8b32..644579c 100644 --- a/src/pike.rs +++ b/src/pike.rs @@ -42,8 +42,8 @@ pub struct Pike { impl Pike { /// Create a new instance of Pike in a given directory pub fn build( - cwd: PathBuf, - cwf: Option, + cwd: &Path, + current_file: Option, mut config_file: Option, ) -> Result { // If no config path is provided, check if the default config file exists @@ -51,22 +51,22 @@ impl Pike { let default_config_file_path = config::default_config_file_path(); if let Ok(default_config_path) = default_config_file_path { if default_config_path.exists() { - config_file = Some(default_config_path.to_path_buf()); + config_file = Some(default_config_path.clone()); } } } let mut workspace = - Workspace::new(&cwd, None).map_err(|e| format!("Error creating workspace: {}", e))?; + Workspace::new(cwd, None).map_err(|e| format!("Error creating workspace: {e}"))?; - if let Some(cwf) = cwf { + if let Some(cwf) = current_file { // Check if file exits, if not, create it if !cwf.exists() { if let Some(parent) = cwf.parent() { fs::create_dir_all(parent) - .map_err(|e| format!("Failed to create directory: {}", e))?; + .map_err(|e| format!("Failed to create directory: {e}"))?; } - File::create(&cwf).map_err(|e| format!("Failed to create file: {}", e))?; + File::create(&cwf).map_err(|e| format!("Failed to create file: {e}"))?; } // Open the given file workspace @@ -76,7 +76,7 @@ impl Pike { Ok(Pike { workspace, config: Config::from_file(config_file.as_deref()) - .map_err(|e| format!("Error loading config: {}", e))?, + .map_err(|e| format!("Error loading config: {e}"))?, cursor_history: CursorHistory::default(), }) } @@ -104,7 +104,7 @@ impl Pike { if !path.exists() { if let Some(parent) = path.parent() { fs::create_dir_all(parent) - .map_err(|e| format!("Failed to create directory: {}", e))?; + .map_err(|e| format!("Failed to create directory: {e}"))?; } File::create(path).map_err(|e| { @@ -196,7 +196,7 @@ impl Pike { pub fn current_buffer_contents(&self) -> String { match self.current_buffer().as_ref() { Some(buffer) => buffer.data(), - None => String::from(""), + None => String::new(), } } @@ -213,9 +213,9 @@ impl Pike { Some(path) => path .file_name() .and_then(|file_name| file_name.to_str()) - .map(|s| s.to_string()) + .map(std::string::ToString::to_string) .expect("Failed to convert filename to string"), - None => String::from(""), + None => String::new(), } } @@ -452,6 +452,7 @@ impl Pike { pub fn save_current_buffer(&mut self) -> Result<(), String> { match &mut self.workspace.current_buffer { Some(buffer) => { + let _ = buffer.data(); buffer.save().expect("Failed to save buffer"); Ok(()) @@ -540,13 +541,13 @@ mod pike_test { let cwd = PathBuf::from(dir.as_path()) .canonicalize() .expect("Failed to canonicalize path"); - let cwf = cwf_content.map(temp_file_with_contents); + let current_file = cwf_content.map(temp_file_with_contents); let config_file = config_content.map(temp_file_with_contents); - let cwf_path = cwf.as_ref().map(|f| f.path().to_path_buf()); + let cwf_path = current_file.as_ref().map(|f| f.path().to_path_buf()); let config_path = config_file.as_ref().map(|f| f.path().to_path_buf()); ( - Pike::build(cwd.clone(), cwf_path, config_path).expect("Failed to build Pike"), + Pike::build(&cwd, cwf_path, config_path).expect("Failed to build Pike"), cwd, ) } @@ -617,10 +618,10 @@ mod pike_test { #[test] fn test_open_file_non_zero_offset() { - let file_contents = r#" + let file_contents = r" Hello, World - "#; + "; let file = temp_file_with_contents(file_contents); let mut pike = tmp_pike_and_working_dir(None, None).0; pike.open_file(file.path(), 1, 2) @@ -647,10 +648,10 @@ mod pike_test { #[test] fn test_open_file_out_of_bounds_offset() { - let file_contents = r#" + let file_contents = r" Hello, World - "#; + "; let file = temp_file_with_contents(file_contents); let mut pike = tmp_pike_and_working_dir(None, None).0; pike.open_file(file.path(), 2, 100) @@ -707,7 +708,7 @@ mod pike_test { } #[test] - #[should_panic] + #[should_panic(expected = "Trying to save a non-existent buffer")] fn test_save_buffer_no_path() { let mut pike = tmp_pike_and_working_dir(None, None).0; pike.open_new_buffer(); @@ -786,9 +787,9 @@ mod pike_test { /// cursor position should be clamped to its length #[test] fn test_move_cursor_down_shorter_line() { - let contents = r#"Hello! + let contents = r"Hello! - This is a test."#; + This is a test."; let (mut pike, _) = tmp_pike_and_working_dir(None, Some(contents)); for _ in 0..5 { pike.move_cursor_right(); @@ -1047,7 +1048,7 @@ mod pike_test { let contents_from_file = fs::read_to_string(file_path).expect("std::fs failed to read from file"); - assert_eq!(file_contents, contents_from_file) + assert_eq!(file_contents, contents_from_file); } #[test] diff --git a/src/test_util.rs b/src/test_util.rs index 2981628..b90eaf6 100644 --- a/src/test_util.rs +++ b/src/test_util.rs @@ -38,7 +38,7 @@ pub mod ui { /// the given index pub fn nth_line_from_terminal_buffer(buf: &Buffer, n: u16) -> String { let width = buf.area.width; - let line = (0..width).fold(String::from(""), |acc, x| { + let line = (0..width).fold(String::new(), |acc, x| { acc + buf .cell::<(u16, u16)>((x, n)) .expect("Iterating from 0 to buf.width should not go out of its bounds") diff --git a/src/ui.rs b/src/ui.rs index f29eb5d..cf8e97e 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -93,17 +93,20 @@ impl UIState { let max_x = { let (x, _) = Self::max_rect_position(area); - x.saturating_sub(border_offset) + x.saturating_sub(border_offset) as usize }; let (base_x, base_y) = { let (x, y) = Self::base_rect_position(area); - (x + border_offset, y) + ((x + border_offset) as usize, y as usize) }; - let offset = input.cursor() as u16; + let offset = input.cursor(); - TerminalPosition::new(min(base_x + offset, max_x), base_y + border_offset) + TerminalPosition::new( + min(base_x + offset, max_x) as u16, + base_y as u16 + border_offset, + ) } /// Calculate position for buffer @@ -118,11 +121,11 @@ impl UIState { let (max_x, max_y) = Self::max_rect_position(area); let (base_x, base_y) = Self::base_rect_position(area); - let x_offset = self.buffer_state.offset.x as u16; - let y_offset = self.buffer_state.offset.y as u16; + let x_offset = self.buffer_state.offset.x; + let y_offset = self.buffer_state.offset.y; - let x = (base_x + cursor_pos.offset as u16).saturating_sub(x_offset); - let y = (base_y + cursor_pos.line as u16).saturating_sub(y_offset); + let x = (base_x + cursor_pos.offset as u16).saturating_sub(x_offset as u16); + let y = (base_y + cursor_pos.line as u16).saturating_sub(y_offset as u16); TerminalPosition { x: min(x, max_x), @@ -227,7 +230,7 @@ impl BufferDisplayState { /// Updates the x offset of the buffer so that the cursor is always visible pub fn update_x_offset(&mut self, area: Rect, cursor_offset_x: usize) { - let too_far_right = cursor_offset_x as u16 >= self.offset.x as u16 + area.width; + let too_far_right = cursor_offset_x >= self.offset.x + area.width as usize; if too_far_right { self.offset.x = cursor_offset_x .saturating_sub(area.width as usize) @@ -240,7 +243,7 @@ impl BufferDisplayState { /// Updates the y offset of the buffer so that the cursor is always visible pub fn update_y_offset(&mut self, area: Rect, cursor_line: usize) { - let too_far_down = cursor_line as u16 >= self.offset.y as u16 + area.height; + let too_far_down = cursor_line >= self.offset.y + area.height as usize; if too_far_down { self.offset.y = cursor_line .saturating_sub(area.height as usize) @@ -264,7 +267,7 @@ impl BufferDisplayState { /// Shifts the content of the buffer to the right by the offset and returns the resulting /// string. Basically, takes every line and removes line[0:self.offset.x] from it, then /// joins and returns them. - fn shift_contents_right(&mut self, contents: String) -> String { + fn shift_contents_right(&mut self, contents: &str) -> String { contents .lines() .map(|line| { @@ -279,7 +282,7 @@ impl BufferDisplayState { /// resulting string. fn shift_contents(&mut self, contents: &str) -> String { let down_shifted = self.shift_contents_down(contents); - self.shift_contents_right(down_shifted) + self.shift_contents_right(&down_shifted) } /// Adds highlights to the given contents and returns a Text widget with the highlights applied. diff --git a/src/welcome_pike.rs b/src/welcome_pike.rs index 92eb5c7..6bb2d6c 100644 --- a/src/welcome_pike.rs +++ b/src/welcome_pike.rs @@ -1,4 +1,4 @@ -pub const WELCOME_MESSAGE: &str = r#" +pub const WELCOME_MESSAGE: &str = r" @@ -31,4 +31,4 @@ pub const WELCOME_MESSAGE: &str = r#" -"#; +"; From 6c0797ef33c9522dfdb7a5eb3f8ef795ec511abe Mon Sep 17 00:00:00 2001 From: Maksym Date: Wed, 10 Dec 2025 13:43:17 +0100 Subject: [PATCH 4/4] fix: test which asserted the wrong panic message, now return an error instead --- src/pike.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/pike.rs b/src/pike.rs index 644579c..fcbe8a8 100644 --- a/src/pike.rs +++ b/src/pike.rs @@ -452,6 +452,9 @@ impl Pike { pub fn save_current_buffer(&mut self) -> Result<(), String> { match &mut self.workspace.current_buffer { Some(buffer) => { + if buffer.path.is_none() { + return Err("Trying to save buffer with no path".to_string()); + } let _ = buffer.data(); buffer.save().expect("Failed to save buffer"); @@ -708,13 +711,11 @@ mod pike_test { } #[test] - #[should_panic(expected = "Trying to save a non-existent buffer")] fn test_save_buffer_no_path() { let mut pike = tmp_pike_and_working_dir(None, None).0; pike.open_new_buffer(); - // This situation should not happen as it's handled in the UI, so a panic here - // is expected - let _ = pike.save_current_buffer(); + // handled in the ui anyway + assert!(pike.save_current_buffer().is_err()); } #[test]