refactor: general logic and style improvements

src/cli.rs

@@ -2,7 +2,6 @@ use std::path::PathBuf;
 
 use crate::{printer::PrintMode, utils};
 use clap::{ArgAction, Parser, Subcommand};
-use thiserror::Error;
 
 #[derive(Parser, Debug)]
 #[command(name = "grip-grab")]
@@ -149,28 +148,24 @@ impl Default for PostProcessedCli {
     }
 }
 
-#[derive(Error, Debug)]
-#[error("Error processing CLI arguments")]
-pub struct CliProcessingError {}
-
-pub fn process_cli_args(mut cli: Cli) -> Result<PostProcessedCli, CliProcessingError> {
+pub fn process_cli_args(mut cli: Cli) -> PostProcessedCli {
     cli.validate();
 
     if cli.paths.is_empty() {
         cli.paths.push(PathBuf::from(DEFAULT_PATH));
     }
 
     if let Some(Commands::Upgrade { force }) = cli.sub_command {
-        return Ok(PostProcessedCli {
+        return PostProcessedCli {
             sub_command: Some(Commands::Upgrade { force }),
             ..Default::default()
-        });
+        };
     }
-    Ok(PostProcessedCli {
-        patterns: if !cli.patterns.is_empty() {
-            cli.patterns
-        } else {
+    PostProcessedCli {
+        patterns: if cli.patterns.is_empty() {
             vec![cli.pattern.unwrap()]
+        } else {
+            cli.patterns
         },
         paths: utils::resolve_paths(cli.paths),
         ignored_paths: utils::resolve_paths(cli.ignore_paths),
@@ -190,5 +185,5 @@ pub fn process_cli_args(mut cli: Cli) -> Result<PostProcessedCli, CliProcessingE
         disable_hyperlinks: cli.disable_hyperlinks,
         disable_devicons: cli.disable_devicons,
         sub_command: cli.sub_command,
-    })
+    }
 }

src/fs.rs

@@ -7,7 +7,7 @@ pub fn walk_builder(
     ignored_paths: &[PathBuf],
     n_threads: usize,
     respect_gitignore: bool,
-    filter_filetypes: Vec<String>,
+    filter_filetypes: &[String],
 ) -> WalkBuilder {
     let mut builder = WalkBuilder::new(paths[0]);
     // add all paths to the builder
@@ -19,15 +19,15 @@ pub fn walk_builder(
     let mut types_builder = TypesBuilder::new();
     types_builder.add_defaults();
     add_custom_filetypes(&mut types_builder).unwrap();
-    filter_filetypes.iter().for_each(|ft| {
+    for ft in filter_filetypes {
         types_builder.select(ft);
-    });
+    }
     builder.types(types_builder.build().unwrap());
 
     // path-based filtering
     let ignored_paths = ignored_paths.to_vec();
     builder.filter_entry(move |entry| {
-        for ignore in ignored_paths.iter() {
+        for ignore in &ignored_paths {
             if entry.path() == ignore {
                 return false;
             }
@@ -43,7 +43,7 @@ pub fn walk_builder(
 }
 
 fn add_custom_filetypes(types_builder: &mut TypesBuilder) -> Result<(), Error> {
-    Ok(types_builder.add("pystrict", "*.py")?)
+    types_builder.add("pystrict", "*.py")
 }
 
 // Original code from https://github.com/BurntSushi/ripgrep/blob/e0f1000df67f82ab0e735bad40e9b45b2d774ef0/crates/cli/src/lib.rs#L249
@@ -58,25 +58,18 @@ pub fn is_readable_stdin() -> bool {
         };
 
         let stdin = std::io::stdin();
-        let fd = match stdin.as_fd().try_clone_to_owned() {
-            Ok(fd) => fd,
-            Err(_) => {
-                return false;
-            }
+        let Ok(fd) = stdin.as_fd().try_clone_to_owned() else {
+            return false;
         };
         let file = File::from(fd);
-        let md = match file.metadata() {
-            Ok(md) => md,
-            Err(_) => {
-                return false;
-            }
+        let Ok(md) = file.metadata() else {
+            return false;
         };
         let ft = md.file_type();
         let is_file = ft.is_file();
         let is_fifo = ft.is_fifo();
         let is_socket = ft.is_socket();
-        let is_readable = is_file || is_fifo || is_socket;
-        is_readable
+        is_file || is_fifo || is_socket
     }
 
     #[cfg(windows)]

src/main.rs

@@ -4,7 +4,7 @@ use std::sync::{mpsc, Arc};
 
 use clap::Parser;
 
-use cli::{CliProcessingError, Commands};
+use cli::Commands;
 use fs::is_readable_stdin;
 use grep::regex::{self, RegexMatcher};
 use ignore::DirEntry;
@@ -27,16 +27,14 @@ mod utils;
 
 #[derive(Error, Debug)]
 pub enum GGError {
-    #[error("Erorr processing CLI arguments")]
-    Cli(#[from] CliProcessingError),
     #[error(transparent)]
     Io(#[from] io::Error),
     #[error(transparent)]
     Regex(#[from] regex::Error),
 }
 
 pub fn main() -> Result<(), GGError> {
-    let cli_args = process_cli_args(Cli::parse())?;
+    let cli_args = process_cli_args(Cli::parse());
 
     if let Some(subcommand) = cli_args.sub_command {
         match subcommand {
@@ -82,7 +80,7 @@ pub fn main() -> Result<(), GGError> {
         &cli_args.ignored_paths,
         cli_args.n_threads,
         !cli_args.disregard_gitignore,
-        cli_args.filter_filetypes,
+        &cli_args.filter_filetypes,
     );
     let matcher: Arc<RegexMatcher> = Arc::new(build_matcher(&cli_args.patterns)?);
 
@@ -95,13 +93,12 @@ pub fn main() -> Result<(), GGError> {
             let tx = tx.clone();
             Box::new(move |entry: Result<DirEntry, ignore::Error>| match entry {
                 Ok(entry) => {
-                    let file_type = entry.file_type().unwrap();
-                    if !file_type.is_dir() {
+                    if !entry.path().is_dir() {
                         let path = entry.path().to_path_buf();
                         match search_file(path, &matcher, &mut searcher) {
                             Ok(file_results) => {
                                 if !file_results.is_empty() {
-                                    tx.send(file_results).unwrap();
+                                    tx.send(file_results).unwrap_or(());
                                 }
                             }
                             Err(_err) => (),

src/printer.rs

@@ -105,8 +105,8 @@ impl ResultsPrinter {
             self.buffer.flush()?;
         }
         match self.config.mode {
-            PrintMode::Text => self.write_colored_text_results(&results.path, results.results),
-            PrintMode::Json => self.writeln_to_buffer(serde_json::to_string(&FileResults {
+            PrintMode::Text => self.write_colored_text_results(&results.path, &results.results),
+            PrintMode::Json => self.writeln_to_buffer(&serde_json::to_string(&FileResults {
                 path: results.path.clone(),
                 results: results.results,
             })?),
@@ -117,7 +117,7 @@ impl ResultsPrinter {
     fn write_colored_text_results(
         &mut self,
         path: &Path,
-        search_results: Vec<SearchResult>,
+        search_results: &[SearchResult],
     ) -> Result<()> {
         self.write_colored_path(path)?;
         self.write_colored_search_results(search_results)?;
@@ -128,7 +128,7 @@ impl ResultsPrinter {
         if !self.config.disable_devicons {
             let icon = FileIcon::from(path);
             self.buffer.set_color(ColorSpec::new().set_fg(Some(
-                devicons_to_termcolor_color(&icon.color).unwrap_or(Color::White),
+                devicons_to_termcolor_color(icon.color).unwrap_or(Color::White),
             )))?;
             write!(&mut self.buffer, "{} ", icon.icon)?;
         }
@@ -142,17 +142,17 @@ impl ResultsPrinter {
             path.to_string_lossy()
         };
         if self.config.disable_hyperlinks {
-            return write!(&mut self.buffer, "{}\n", display_path);
+            return writeln!(&mut self.buffer, "{display_path}");
         }
         let path_str = path.to_string_lossy();
         let link = Hyperlink {
-            uri: &format!("file://{}", path_str),
+            uri: &format!("file://{path_str}"),
             id: None,
         };
-        write!(&mut self.buffer, "{link}{}{link:#}\n", display_path)
+        writeln!(&mut self.buffer, "{link}{display_path}{link:#}",)
     }
 
-    fn write_colored_search_results(&mut self, results: Vec<SearchResult>) -> Result<()> {
+    fn write_colored_search_results(&mut self, results: &[SearchResult]) -> Result<()> {
         results.iter().try_for_each(|result| {
             self.write_colored_line(result)?;
             Ok(())
@@ -191,12 +191,14 @@ impl ResultsPrinter {
         write!(&mut self.buffer, "{}", &result.line[last_end_offset..])
     }
 
-    fn writeln_to_buffer(&mut self, text: String) -> Result<()> {
-        writeln!(self.buffer, "{}", text)
+    fn writeln_to_buffer(&mut self, text: &str) -> Result<()> {
+        writeln!(self.buffer, "{text}")
     }
 
+    const EMPTY_STRING: &str = "";
+
     fn write_newline_to_buffer(&mut self) -> Result<()> {
-        writeln!(self.buffer, "")
+        writeln!(self.buffer, "{}", Self::EMPTY_STRING)
     }
 
     pub fn wipeout(&mut self) -> Result<()> {
@@ -213,9 +215,13 @@ impl ResultsPrinter {
 
 fn devicons_to_termcolor_color(d_color: &str) -> Option<Color> {
     d_color.strip_prefix("#").and_then(|hex| {
-        u32::from_str_radix(hex, 16)
-            .ok()
-            .map(|c| Color::Rgb((c >> 16) as u8, (c >> 8) as u8, c as u8))
+        if hex.len() != 6 {
+            return None;
+        }
+        let red = u8::from_str_radix(&hex[0..2], 16).ok()?;
+        let green = u8::from_str_radix(&hex[2..4], 16).ok()?;
+        let blue = u8::from_str_radix(&hex[4..6], 16).ok()?;
+        Some(Color::Rgb(red, green, blue))
     })
 }
 

src/search.rs

@@ -1,3 +1,4 @@
+#![allow(clippy::module_name_repetitions)]
 use std::{fmt, io};
 use std::{path::PathBuf, slice::Iter};
 
@@ -148,7 +149,7 @@ pub struct FileResults {
 
 impl fmt::Display for FileResults {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}\n", self.path.to_string_lossy()).and_then(|_| {
+        writeln!(f, "{}\n", self.path.to_string_lossy()).and_then(|()| {
             self.results
                 .iter()
                 .try_for_each(|r| write!(f, "{}: {}", r.line_number, r.line))
@@ -181,15 +182,15 @@ struct PartialSearchResult {
     pub m: MatchRange,
 }
 
-pub fn search_file<'a>(
+pub fn search_file(
     path: PathBuf,
     matcher: &RegexMatcher,
     searcher: &mut Searcher,
 ) -> Result<FileResults, io::Error> {
     let mut partial_results: Vec<PartialSearchResult> = Vec::new();
 
     searcher.search_path(
-        &matcher,
+        matcher,
         &path,
         UTF8(|lnum, line| {
             matcher.find_iter(line.as_bytes(), |m| {
@@ -217,27 +218,26 @@ pub fn search_file<'a>(
         line_end: partial_results[0].line_number,
         matches: vec![partial_results[0].m.clone()],
     }];
-    for partial_result in partial_results[1..].iter() {
+    for partial_result in &partial_results[1..] {
         let last_result = results.last_mut().unwrap();
-        if last_result.line_number != partial_result.line_number {
+        if last_result.line_number == partial_result.line_number {
+            last_result.matches.push(partial_result.m.clone());
+            last_result.line_end = partial_result.line_number;
+        } else {
             results.push(SearchResult {
                 line_number: partial_result.line_number,
                 line: partial_result.line.clone(),
                 line_start: partial_result.line_number,
                 line_end: partial_result.line_number,
                 matches: vec![partial_result.m.clone()],
             });
-        } else {
-            last_result.matches.push(partial_result.m.clone());
-            last_result.line_end = partial_result.line_number;
         }
     }
 
     Ok(FileResults { path, results })
 }
 
-// io::Error
-// std::fmt::Display
+#[allow(clippy::similar_names)]
 pub fn search_reader(
     reader: impl std::io::BufRead,
     matcher: &RegexMatcher,
@@ -268,10 +268,10 @@ pub fn search_reader(
     Ok(results)
 }
 
-pub fn build_matcher(patterns: &Vec<String>) -> Result<RegexMatcher, regex::Error> {
+pub fn build_matcher(patterns: &[String]) -> Result<RegexMatcher, regex::Error> {
     let builder = RegexMatcherBuilder::new();
     // matcher Error
-    Ok(builder.build_many(patterns)?)
+    builder.build_many(patterns)
 }
 
 pub fn build_searcher(multiline: bool) -> Searcher {

src/upgrade.rs

@@ -64,8 +64,8 @@ pub fn upgrade_gg(force: bool) {
         let reader = BufReader::new(stdout);
         for line in reader.lines() {
             match line {
-                Ok(line) => println!("{}", line),
-                Err(e) => eprintln!("Error reading stdout: {}", e),
+                Ok(line) => println!("{line}"),
+                Err(e) => eprintln!("Error reading stdout: {e}"),
             }
         }
     });
@@ -74,8 +74,8 @@ pub fn upgrade_gg(force: bool) {
         let reader = BufReader::new(stderr);
         for line in reader.lines() {
             match line {
-                Ok(line) => eprintln!("{}", line),
-                Err(e) => eprintln!("Error reading stderr: {}", e),
+                Ok(line) => eprintln!("{line}"),
+                Err(e) => eprintln!("Error reading stderr: {e}"),
             }
         }
     });

src/utils.rs

@@ -1,9 +1,9 @@
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 pub fn resolve_paths(paths: Vec<PathBuf>) -> Vec<PathBuf> {
-    paths.into_iter().map(|path| resolve_path(path)).collect()
+    paths.into_iter().map(|pb| resolve_path(&pb)).collect()
 }
 
-pub fn resolve_path(path: PathBuf) -> PathBuf {
+pub fn resolve_path(path: &Path) -> PathBuf {
     path.canonicalize().unwrap()
 }