From 35347c23109400e9694f073bf8fe901b3ca1d59f Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 2 Mar 2021 11:15:49 +0300 Subject: [PATCH] Improve readability Using `Path`s for paths expresses intent more clearly. --- CHANGELOG.md | 2 ++ src/assets.rs | 8 ++++---- src/bin/bat/app.rs | 17 ++++++++++------- src/bin/bat/input.rs | 8 ++++---- src/bin/bat/main.rs | 3 +-- src/diff.rs | 3 +-- src/input.rs | 20 +++++++++++++------- src/pretty_printer.rs | 14 +++++++------- 8 files changed, 42 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01209209..5d36e2f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ ## Other +- `Input::ordinary_file` and `Input::with_name` now accept `Path` rather than `OsStr` see #1571 (@matklad) + ## Syntaxes ## New themes diff --git a/src/assets.rs b/src/assets.rs index 48f59da9..19650a27 100644 --- a/src/assets.rs +++ b/src/assets.rs @@ -327,7 +327,7 @@ mod tests { writeln!(temp_file, "{}", first_line).unwrap(); } - let input = Input::ordinary_file(file_path.as_os_str()); + let input = Input::ordinary_file(&file_path); let dummy_stdin: &[u8] = &[]; let mut opened_input = input.open(dummy_stdin, None).unwrap(); @@ -341,7 +341,7 @@ mod tests { fn syntax_for_file_with_content_os(&self, file_name: &OsStr, first_line: &str) -> String { let file_path = self.temp_dir.path().join(file_name); let input = Input::from_reader(Box::new(BufReader::new(first_line.as_bytes()))) - .with_name(Some(file_path.as_os_str())); + .with_name(Some(&file_path)); let dummy_stdin: &[u8] = &[]; let mut opened_input = input.open(dummy_stdin, None).unwrap(); @@ -366,7 +366,7 @@ mod tests { } fn syntax_for_stdin_with_content(&self, file_name: &str, content: &[u8]) -> String { - let input = Input::stdin().with_name(Some(OsStr::new(file_name))); + let input = Input::stdin().with_name(Some(file_name)); let mut opened_input = input.open(content, None).unwrap(); self.assets @@ -521,7 +521,7 @@ mod tests { .expect("creation of directory succeeds"); symlink(&file_path, &file_path_symlink).expect("creation of symbolic link succeeds"); - let input = Input::ordinary_file(file_path_symlink.as_os_str()); + let input = Input::ordinary_file(&file_path_symlink); let dummy_stdin: &[u8] = &[]; let mut opened_input = input.open(dummy_stdin, None).unwrap(); diff --git a/src/bin/bat/app.rs b/src/bin/bat/app.rs index 34f3f76c..ce27b35c 100644 --- a/src/bin/bat/app.rs +++ b/src/bin/bat/app.rs @@ -1,6 +1,6 @@ use std::collections::HashSet; use std::env; -use std::ffi::OsStr; +use std::path::Path; use std::str::FromStr; use atty::{self, Stream}; @@ -248,16 +248,19 @@ impl App { } _ => {} } - let filenames: Option> = self + let filenames: Option> = self .matches - .values_of("file-name") - .map(|values| values.collect()); + .values_of_os("file-name") + .map(|values| values.map(Path::new).collect()); - let mut filenames_or_none: Box> = match filenames { - Some(ref filenames) => Box::new(filenames.iter().map(|name| Some(OsStr::new(*name)))), + let mut filenames_or_none: Box>> = match filenames { + Some(filenames) => Box::new(filenames.into_iter().map(Some)), None => Box::new(std::iter::repeat(None)), }; - let files: Option> = self.matches.values_of_os("FILE").map(|vs| vs.collect()); + let files: Option> = self + .matches + .values_of_os("FILE") + .map(|vs| vs.map(Path::new).collect()); if files.is_none() { return Ok(vec![new_stdin_input( diff --git a/src/bin/bat/input.rs b/src/bin/bat/input.rs index 8e65d800..3c928906 100644 --- a/src/bin/bat/input.rs +++ b/src/bin/bat/input.rs @@ -1,15 +1,15 @@ use bat::input::Input; -use std::ffi::OsStr; +use std::path::Path; -pub fn new_file_input<'a>(file: &'a OsStr, name: Option<&'a OsStr>) -> Input<'a> { +pub fn new_file_input<'a>(file: &'a Path, name: Option<&'a Path>) -> Input<'a> { named(Input::ordinary_file(file), name.or(Some(file))) } -pub fn new_stdin_input(name: Option<&OsStr>) -> Input { +pub fn new_stdin_input(name: Option<&Path>) -> Input { named(Input::stdin(), name) } -fn named<'a>(input: Input<'a>, name: Option<&OsStr>) -> Input<'a> { +fn named<'a>(input: Input<'a>, name: Option<&Path>) -> Input<'a> { if let Some(provided_name) = name { let mut input = input.with_name(Some(provided_name)); input.description_mut().set_kind(Some("File".to_owned())); diff --git a/src/bin/bat/main.rs b/src/bin/bat/main.rs index c36d369b..eb047809 100644 --- a/src/bin/bat/main.rs +++ b/src/bin/bat/main.rs @@ -9,7 +9,6 @@ mod directories; mod input; use std::collections::{HashMap, HashSet}; -use std::ffi::OsStr; use std::io; use std::io::{BufReader, Write}; use std::path::Path; @@ -269,7 +268,7 @@ fn run() -> Result { run_cache_subcommand(cache_matches)?; Ok(true) } else { - let inputs = vec![Input::ordinary_file(OsStr::new("cache"))]; + let inputs = vec![Input::ordinary_file("cache")]; let config = app.config(&inputs)?; run_controller(inputs, &config) diff --git a/src/diff.rs b/src/diff.rs index bd08468a..1aaca4f4 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -1,7 +1,6 @@ #![cfg(feature = "git")] use std::collections::HashMap; -use std::ffi::OsStr; use std::fs; use std::path::Path; @@ -17,7 +16,7 @@ pub enum LineChange { pub type LineChanges = HashMap; -pub fn get_git_diff(filename: &OsStr) -> Option { +pub fn get_git_diff(filename: &Path) -> Option { let repo = Repository::discover(&filename).ok()?; let repo_path_absolute = fs::canonicalize(repo.workdir()?).ok()?; diff --git a/src/input.rs b/src/input.rs index 0cc902cd..f31b404b 100644 --- a/src/input.rs +++ b/src/input.rs @@ -1,7 +1,7 @@ use std::convert::TryFrom; -use std::ffi::{OsStr, OsString}; use std::fs::File; use std::io::{self, BufRead, BufReader, Read}; +use std::path::{Path, PathBuf}; use clircle::{Clircle, Identifier}; use content_inspector::{self, ContentType}; @@ -69,7 +69,7 @@ impl InputDescription { } pub(crate) enum InputKind<'a> { - OrdinaryFile(OsString), + OrdinaryFile(PathBuf), StdIn, CustomReader(Box), } @@ -86,7 +86,7 @@ impl<'a> InputKind<'a> { #[derive(Clone, Default)] pub(crate) struct InputMetadata { - pub(crate) user_provided_name: Option, + pub(crate) user_provided_name: Option, } pub struct Input<'a> { @@ -96,7 +96,7 @@ pub struct Input<'a> { } pub(crate) enum OpenedInputKind { - OrdinaryFile(OsString), + OrdinaryFile(PathBuf), StdIn, CustomReader, } @@ -109,8 +109,11 @@ pub(crate) struct OpenedInput<'a> { } impl<'a> Input<'a> { - pub fn ordinary_file(path: &OsStr) -> Self { - let kind = InputKind::OrdinaryFile(path.to_os_string()); + pub fn ordinary_file(path: impl AsRef) -> Self { + Self::_ordinary_file(path.as_ref()) + } + fn _ordinary_file(path: &Path) -> Self { + let kind = InputKind::OrdinaryFile(path.to_path_buf()); Input { description: kind.description(), metadata: InputMetadata::default(), @@ -140,7 +143,10 @@ impl<'a> Input<'a> { matches!(self.kind, InputKind::StdIn) } - pub fn with_name(mut self, provided_name: Option<&OsStr>) -> Self { + pub fn with_name(mut self, provided_name: Option>) -> Self { + self._with_name(provided_name.as_ref().map(|it| it.as_ref())) + } + pub fn _with_name(mut self, provided_name: Option<&Path>) -> Self { if let Some(name) = provided_name { self.description.name = name.to_string_lossy().to_string() } diff --git a/src/pretty_printer.rs b/src/pretty_printer.rs index 44da3283..7f1c2b7f 100644 --- a/src/pretty_printer.rs +++ b/src/pretty_printer.rs @@ -1,5 +1,5 @@ -use std::ffi::OsStr; use std::io::Read; +use std::path::Path; use console::Term; use syntect::parsing::SyntaxReference; @@ -71,7 +71,7 @@ impl<'a> PrettyPrinter<'a> { } /// Add a file which should be pretty-printed - pub fn input_file(&mut self, path: impl AsRef) -> &mut Self { + pub fn input_file(&mut self, path: impl AsRef) -> &mut Self { self.input(Input::from_file(path).kind("File")) } @@ -79,7 +79,7 @@ impl<'a> PrettyPrinter<'a> { pub fn input_files(&mut self, paths: I) -> &mut Self where I: IntoIterator, - P: AsRef, + P: AsRef, { self.inputs(paths.into_iter().map(Input::from_file)) } @@ -297,8 +297,8 @@ impl<'a> Input<'a> { } /// A new input from a file. - pub fn from_file(path: impl AsRef) -> Self { - input::Input::ordinary_file(path.as_ref()).into() + pub fn from_file(path: impl AsRef) -> Self { + input::Input::ordinary_file(path).into() } /// A new input from bytes. @@ -313,8 +313,8 @@ impl<'a> Input<'a> { /// The filename of the input. /// This affects syntax detection and changes the default header title. - pub fn name(mut self, name: impl AsRef) -> Self { - self.input = self.input.with_name(Some(name.as_ref())); + pub fn name(mut self, name: impl AsRef) -> Self { + self.input = self.input.with_name(Some(name)); self }