From e37e9c12141a40fc65c207a501dad24f11709ee0 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 24 Apr 2020 13:46:01 +0700 Subject: [PATCH] Fix some clippy lints Some might actually improve perf --- src/assets.rs | 6 +++--- src/assets_metadata.rs | 2 +- src/bin/bat/app.rs | 10 ++++++---- src/bin/bat/assets.rs | 2 +- src/bin/bat/clap_app.rs | 2 +- src/bin/bat/config.rs | 6 +++--- src/bin/bat/main.rs | 2 +- src/controller.rs | 4 ++-- src/line_range.rs | 2 +- src/preprocessor.rs | 13 +++++-------- src/printer.rs | 12 +++++------- tests/no_duplicate_extensions.rs | 2 +- 12 files changed, 30 insertions(+), 33 deletions(-) diff --git a/src/assets.rs b/src/assets.rs index 7831b56c..4f8556f0 100644 --- a/src/assets.rs +++ b/src/assets.rs @@ -180,7 +180,7 @@ impl HighlightingAssets { theme ); } - &self.theme_set.themes[self.fallback_theme.unwrap_or(Self::default_theme())] + &self.theme_set.themes[self.fallback_theme.unwrap_or_else(|| Self::default_theme())] } } } @@ -204,7 +204,7 @@ impl HighlightingAssets { let path = Path::new(path_str); let line_syntax = self.get_first_line_syntax(&mut input.reader); - let absolute_path = path.canonicalize().ok().unwrap_or(path.to_owned()); + let absolute_path = path.canonicalize().ok().unwrap_or_else(|| path.to_owned()); match mapping.get_syntax_for(absolute_path) { Some(MappingTarget::MapTo(syntax_name)) => { // TODO: we should probably return an error here if this syntax can not be @@ -221,7 +221,7 @@ impl HighlightingAssets { OpenedInputKind::StdIn | OpenedInputKind::CustomReader => { if let Some(ref name) = input.metadata.user_provided_name { self.get_extension_syntax(&name) - .or(self.get_first_line_syntax(&mut input.reader)) + .or_else(|| self.get_first_line_syntax(&mut input.reader)) } else { self.get_first_line_syntax(&mut input.reader) } diff --git a/src/assets_metadata.rs b/src/assets_metadata.rs index da0ed88f..91fc7679 100644 --- a/src/assets_metadata.rs +++ b/src/assets_metadata.rs @@ -13,7 +13,7 @@ pub struct AssetsMetadata { creation_time: Option, } -const FILENAME: &'static str = "metadata.yaml"; +const FILENAME: &str = "metadata.yaml"; impl AssetsMetadata { pub(crate) fn new(current_version: &str) -> AssetsMetadata { diff --git a/src/bin/bat/app.rs b/src/bin/bat/app.rs index f0f519ea..d01f756d 100644 --- a/src/bin/bat/app.rs +++ b/src/bin/bat/app.rs @@ -80,6 +80,7 @@ impl App { let paging_mode = match self.matches.value_of("paging") { Some("always") => PagingMode::Always, Some("never") => PagingMode::Never, + // FIXME: `_` will always cover in or patterns Some("auto") | _ => { if self.matches.occurrences_of("plain") > 1 { // If we have -pp as an option when in auto mode, the pager should be disabled. @@ -147,6 +148,7 @@ impl App { match self.matches.value_of("wrap") { Some("character") => WrappingMode::Character, Some("never") => WrappingMode::NoWrapping, + // FIXME: `_` will always cover in or patterns Some("auto") | _ => { if style_components.plain() { WrappingMode::NoWrapping @@ -249,18 +251,18 @@ impl App { let mut filenames_or_none: Box> = match filenames { Some(ref filenames) => { - Box::new(filenames.into_iter().map(|name| Some(OsStr::new(*name)))) + Box::new(filenames.iter().map(|name| Some(OsStr::new(*name)))) } None => Box::new(std::iter::repeat(None)), }; let files: Option> = self.matches.values_of_os("FILE").map(|vs| vs.collect()); if files.is_none() { - let input = Input::stdin().with_name(filenames_or_none.nth(0).unwrap_or(None)); + let input = Input::stdin().with_name(filenames_or_none.next().unwrap_or(None)); return Ok(vec![input]); } let files_or_none: Box> = match files { - Some(ref files) => Box::new(files.into_iter().map(|name| Some(*name))), + Some(ref files) => Box::new(files.iter().map(|name| Some(*name))), None => Box::new(std::iter::repeat(None)), }; @@ -274,7 +276,7 @@ impl App { } } } - return Ok(file_input); + Ok(file_input) } fn style_components(&self) -> Result { diff --git a/src/bin/bat/assets.rs b/src/bin/bat/assets.rs index c0abfb90..ead0b7cb 100644 --- a/src/bin/bat/assets.rs +++ b/src/bin/bat/assets.rs @@ -53,5 +53,5 @@ pub fn assets_from_cache_or_binary() -> Result { } } - Ok(HighlightingAssets::from_cache(&cache_dir).unwrap_or(HighlightingAssets::from_binary())) + Ok(HighlightingAssets::from_cache(&cache_dir).unwrap_or_else(|_| HighlightingAssets::from_binary())) } diff --git a/src/bin/bat/clap_app.rs b/src/bin/bat/clap_app.rs index 85edefde..01a5bf3d 100644 --- a/src/bin/bat/clap_app.rs +++ b/src/bin/bat/clap_app.rs @@ -180,7 +180,7 @@ pub fn build_app(interactive_output: bool) -> ClapApp<'static, 'static> { t.parse::() .map_err(|_e| "must be an offset or number") .and_then(|v| if v == 0 && !is_offset { - Err("terminal width cannot be zero".into()) + Err("terminal width cannot be zero") } else { Ok(()) }) diff --git a/src/bin/bat/config.rs b/src/bin/bat/config.rs index 38bac82f..9b8c8925 100644 --- a/src/bin/bat/config.rs +++ b/src/bin/bat/config.rs @@ -35,10 +35,10 @@ pub fn generate_config_file() -> bat::error::Result<()> { match config_dir { Some(path) => fs::create_dir_all(path)?, None => { - return Ok(Err(format!( + return Err(format!( "Unable to write config file to: {}", config_file.to_string_lossy() - ))?) + ).into()); } } } @@ -77,7 +77,7 @@ pub fn generate_config_file() -> bat::error::Result<()> { config_file.to_string_lossy() ); - return Ok(()); + Ok(()) } pub fn get_args_from_config_file() -> Result, shell_words::ParseError> { diff --git a/src/bin/bat/main.rs b/src/bin/bat/main.rs index 818286d6..01575cc5 100644 --- a/src/bin/bat/main.rs +++ b/src/bin/bat/main.rs @@ -70,7 +70,7 @@ pub fn list_languages(config: &Config) -> Result<()> { if config.loop_through { for lang in languages { - write!(stdout, "{}:{}\n", lang.name, lang.file_extensions.join(","))?; + writeln!(stdout, "{}:{}", lang.name, lang.file_extensions.join(","))?; } } else { let longest = languages diff --git a/src/controller.rs b/src/controller.rs index f636d5fd..45156a26 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -45,9 +45,9 @@ impl<'b> Controller<'b> { if self.config.paging_mode != PagingMode::Never { let call_pager = inputs.iter().any(|ref input| { if let InputKind::OrdinaryFile(ref path) = input.kind { - return Path::new(path).exists(); + Path::new(path).exists() } else { - return true; + true } }); if !call_pager { diff --git a/src/line_range.rs b/src/line_range.rs index ba855fcb..66046500 100644 --- a/src/line_range.rs +++ b/src/line_range.rs @@ -30,7 +30,7 @@ impl LineRange { fn parse_range(range_raw: &str) -> Result { let mut new_range = LineRange::default(); - if range_raw.bytes().nth(0).ok_or("Empty line range")? == b':' { + if range_raw.bytes().next().ok_or("Empty line range")? == b':' { new_range.upper = range_raw[1..].parse()?; return Ok(new_range); } else if range_raw.bytes().last().ok_or("Empty line range")? == b':' { diff --git a/src/preprocessor.rs b/src/preprocessor.rs index cfe0ffd6..373efc2f 100644 --- a/src/preprocessor.rs +++ b/src/preprocessor.rs @@ -36,15 +36,12 @@ pub fn expand_tabs(line: &str, width: usize, cursor: &mut usize) -> String { fn try_parse_utf8_char(input: &[u8]) -> Option<(char, usize)> { let str_from_utf8 = |seq| std::str::from_utf8(seq).ok(); - let decoded = None - .or(input.get(0..1).and_then(str_from_utf8).map(|c| (c, 1))) - .or(input.get(0..2).and_then(str_from_utf8).map(|c| (c, 2))) - .or(input.get(0..3).and_then(str_from_utf8).map(|c| (c, 3))) - .or(input.get(0..4).and_then(str_from_utf8).map(|c| (c, 4))); + let decoded = input.get(0..1).and_then(str_from_utf8).map(|c| (c, 1)) + .or_else(|| input.get(0..2).and_then(str_from_utf8).map(|c| (c, 2))) + .or_else(|| input.get(0..3).and_then(str_from_utf8).map(|c| (c, 3))) + .or_else(|| input.get(0..4).and_then(str_from_utf8).map(|c| (c, 4))); - let decoded_char = decoded.map(|(seq, n)| (seq.chars().next().unwrap(), n)); - - decoded_char + decoded.map(|(seq, n)| (seq.chars().next().unwrap(), n)) } pub fn replace_nonprintable(input: &[u8], tab_width: usize) -> String { diff --git a/src/printer.rs b/src/printer.rs index 2b245cfd..09bf4ecf 100644 --- a/src/printer.rs +++ b/src/printer.rs @@ -202,7 +202,7 @@ impl<'a> InteractivePrinter<'a> { if self.config.style_components.grid() { format!("{} │ ", text_filled) } else { - format!("{}", text_filled) + text_filled } } } @@ -229,10 +229,8 @@ impl<'a> Printer for InteractivePrinter<'a> { Yellow.paint("[bat warning]"), input.description().full, )?; - } else { - if self.config.style_components.grid() { - self.print_horizontal_line(handle, '┬')?; - } + } else if self.config.style_components.grid() { + self.print_horizontal_line(handle, '┬')?; } return Ok(()); } @@ -304,9 +302,9 @@ impl<'a> Printer for InteractivePrinter<'a> { let snip_right = " ─".repeat((self.config.term_width - panel_count - snip_left_count - title_count) / 2); - write!( + writeln!( handle, - "{}\n", + "{}", self.colors .grid .paint(format!("{}{}{}{}", panel, snip_left, title, snip_right)) diff --git a/tests/no_duplicate_extensions.rs b/tests/no_duplicate_extensions.rs index cdc96128..62cfa663 100644 --- a/tests/no_duplicate_extensions.rs +++ b/tests/no_duplicate_extensions.rs @@ -4,7 +4,7 @@ use bat::assets::HighlightingAssets; #[test] fn no_duplicate_extensions() { - const KNOWN_EXCEPTIONS: &[&'static str] = &[ + const KNOWN_EXCEPTIONS: &[&str] = &[ // The '.h' extension currently appears in multiple syntaxes: C, C++, Objective C, // Objective C++ "h",