Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add small refactorings #1847

Merged
merged 4 commits into from
Sep 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 47 additions & 48 deletions src/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,57 +255,56 @@ impl HighlightingAssets {
) -> Result<SyntaxReferenceInSet> {
if let Some(language) = language {
let syntax_set = self.get_syntax_set_by_name(language)?;
syntax_set
return syntax_set
.find_syntax_by_token(language)
.map(|syntax| SyntaxReferenceInSet { syntax, syntax_set })
.ok_or_else(|| Error::UnknownSyntax(language.to_owned()))
} else {
let line_syntax = self.get_first_line_syntax(&mut input.reader)?;

// Get the path of the file:
// If this was set by the metadata, that will take priority.
// If it wasn't, it will use the real file path (if available).
let path_str =
input
.metadata
.user_provided_name
.as_ref()
.or_else(|| match input.kind {
OpenedInputKind::OrdinaryFile(ref path) => Some(path),
_ => None,
});

if let Some(path_str) = path_str {
// If a path was provided, we try and detect the syntax based on extension mappings.
let path = Path::new(path_str);
let absolute_path = PathAbs::new(path)
.ok()
.map(|p| p.as_path().to_path_buf())
.unwrap_or_else(|| path.to_owned());

match mapping.get_syntax_for(absolute_path) {
Some(MappingTarget::MapToUnknown) => line_syntax
.ok_or_else(|| Error::UndetectedSyntax(path.to_string_lossy().into())),

Some(MappingTarget::MapTo(syntax_name)) => {
let syntax_set = self.get_syntax_set()?;
syntax_set
.find_syntax_by_name(syntax_name)
.map(|syntax| SyntaxReferenceInSet { syntax, syntax_set })
.ok_or_else(|| Error::UnknownSyntax(syntax_name.to_owned()))
}

None => {
let file_name = path.file_name().unwrap_or_default();
self.get_extension_syntax(file_name)?
.or(line_syntax)
.ok_or_else(|| Error::UndetectedSyntax(path.to_string_lossy().into()))
}
.ok_or_else(|| Error::UnknownSyntax(language.to_owned()));
}

let line_syntax = self.get_first_line_syntax(&mut input.reader)?;

// Get the path of the file:
// If this was set by the metadata, that will take priority.
// If it wasn't, it will use the real file path (if available).
let path_str = input
.metadata
.user_provided_name
.as_ref()
.or_else(|| match input.kind {
OpenedInputKind::OrdinaryFile(ref path) => Some(path),
_ => None,
});

if let Some(path_str) = path_str {
// If a path was provided, we try and detect the syntax based on extension mappings.
let path = Path::new(path_str);
let absolute_path = PathAbs::new(path)
.ok()
.map(|p| p.as_path().to_path_buf())
.unwrap_or_else(|| path.to_owned());

match mapping.get_syntax_for(absolute_path) {
Some(MappingTarget::MapToUnknown) => line_syntax
.ok_or_else(|| Error::UndetectedSyntax(path.to_string_lossy().into())),

Some(MappingTarget::MapTo(syntax_name)) => {
let syntax_set = self.get_syntax_set()?;
syntax_set
.find_syntax_by_name(syntax_name)
.map(|syntax| SyntaxReferenceInSet { syntax, syntax_set })
.ok_or_else(|| Error::UnknownSyntax(syntax_name.to_owned()))
}

None => {
let file_name = path.file_name().unwrap_or_default();
self.get_extension_syntax(file_name)?
.or(line_syntax)
.ok_or_else(|| Error::UndetectedSyntax(path.to_string_lossy().into()))
}
} else {
// If a path wasn't provided, we fall back to the detect first-line syntax.
line_syntax.ok_or_else(|| Error::UndetectedSyntax("[unknown]".into()))
}
} else {
// If a path wasn't provided, we fall back to the detect first-line syntax.
line_syntax.ok_or_else(|| Error::UndetectedSyntax("[unknown]".into()))
}
}

Expand Down Expand Up @@ -352,7 +351,7 @@ impl HighlightingAssets {
let file_path = Path::new(file_name);
let mut syntax = None;
if let Some(file_str) = file_path.to_str() {
for suffix in IGNORED_SUFFIXES.iter() {
for suffix in &IGNORED_SUFFIXES {
if let Some(stripped_filename) = file_str.strip_suffix(suffix) {
syntax = self.get_extension_syntax(OsStr::new(stripped_filename))?;
break;
Expand Down
21 changes: 9 additions & 12 deletions src/bin/bat/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub fn get_languages(config: &Config) -> Result<String> {
.collect::<Vec<_>>();

// Handling of file-extension conflicts, see issue #1076
for lang in languages.iter_mut() {
for lang in &mut languages {
let lang_name = lang.name.clone();
lang.file_extensions.retain(|extension| {
// The 'extension' variable is not certainly a real extension.
Expand All @@ -100,25 +100,22 @@ pub fn get_languages(config: &Config) -> Result<String> {
// Also skip if the 'extension' contains another real extension, likely
// that is a full match file name like 'CMakeLists.txt' and 'Cargo.lock'
if extension.starts_with('.') || Path::new(extension).extension().is_some() {
true
} else {
let test_file = Path::new("test").with_extension(extension);
let syntax_in_set = assets
.get_syntax_for_file_name(test_file, &config.syntax_mapping)
.unwrap(); // safe since .get_syntaxes() above worked
match syntax_in_set {
Some(syntax_in_set) => syntax_in_set.syntax.name == lang_name,
None => false,
}
return true;
}

let test_file = Path::new("test").with_extension(extension);
let syntax_in_set = assets
.get_syntax_for_file_name(test_file, &config.syntax_mapping)
.unwrap(); // safe since .get_syntaxes() above worked
matches!(syntax_in_set, Some(syntax_in_set) if syntax_in_set.syntax.name == lang_name)
});
}

languages.sort_by_key(|lang| lang.name.to_uppercase());

let configured_languages = get_syntax_mapping_to_paths(config.syntax_mapping.mappings());

for lang in languages.iter_mut() {
for lang in &mut languages {
if let Some(additional_paths) = configured_languages.get(lang.name.as_str()) {
lang.file_extensions
.extend(additional_paths.iter().cloned());
Expand Down
9 changes: 4 additions & 5 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,10 @@ pub struct Config<'a> {

#[cfg(all(feature = "minimal-application", feature = "paging"))]
pub fn get_pager_executable(config_pager: Option<&str>) -> Option<String> {
if let Ok(Some(pager)) = crate::pager::get_pager(config_pager) {
Some(pager.bin)
} else {
None
}
crate::pager::get_pager(config_pager)
.ok()
.flatten()
.map(|pager| pager.bin)
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ impl<'b> Controller<'b> {
let mut line_ranges: Vec<LineRange> = vec![];

if let Some(line_changes) = line_changes {
for line in line_changes.keys() {
let line = *line as usize;
for &line in line_changes.keys() {
let line = line as usize;
line_ranges.push(LineRange::new(line - context, line + context));
}
}
Expand Down
20 changes: 10 additions & 10 deletions src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl InputDescription {
}

pub fn title(&self) -> &String {
match self.title.as_ref() {
match &self.title {
Some(title) => title,
None => &self.name,
}
Expand Down Expand Up @@ -256,18 +256,18 @@ impl<'a> InputReader<'a> {
}

pub(crate) fn read_line(&mut self, buf: &mut Vec<u8>) -> io::Result<bool> {
if self.first_line.is_empty() {
let res = self.inner.read_until(b'\n', buf).map(|size| size > 0)?;
if !self.first_line.is_empty() {
buf.append(&mut self.first_line);
return Ok(true);
}

if self.content_type == Some(ContentType::UTF_16LE) {
self.inner.read_until(0x00, buf).ok();
}
let res = self.inner.read_until(b'\n', buf).map(|size| size > 0)?;

Ok(res)
} else {
buf.append(&mut self.first_line);
Ok(true)
if self.content_type == Some(ContentType::UTF_16LE) {
let _ = self.inner.read_until(0x00, buf);
}

Ok(res)
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/less.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ pub fn retrieve_less_version(less_path: &dyn AsRef<OsStr>) -> Option<usize> {
}

fn parse_less_version(output: &[u8]) -> Option<usize> {
if output.starts_with(b"less ") {
let version = std::str::from_utf8(&output[5..]).ok()?;
let end = version.find(|c: char| !c.is_ascii_digit())?;
version[..end].parse::<usize>().ok()
} else {
None
if !output.starts_with(b"less ") {
return None;
}

let version = std::str::from_utf8(&output[5..]).ok()?;
let end = version.find(|c: char| !c.is_ascii_digit())?;
version[..end].parse::<usize>().ok()
}

#[test]
Expand Down
41 changes: 20 additions & 21 deletions src/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,29 +226,29 @@ impl<'a> InteractivePrinter<'a> {

fn create_fake_panel(&self, text: &str) -> String {
if self.panel_width == 0 {
"".to_string()
return "".to_string();
}

let text_truncated: String = text.chars().take(self.panel_width - 1).collect();
let text_filled: String = format!(
"{}{}",
text_truncated,
" ".repeat(self.panel_width - 1 - text_truncated.len())
);
if self.config.style_components.grid() {
format!("{} │ ", text_filled)
} else {
let text_truncated: String = text.chars().take(self.panel_width - 1).collect();
let text_filled: String = format!(
"{}{}",
text_truncated,
" ".repeat(self.panel_width - 1 - text_truncated.len())
);
if self.config.style_components.grid() {
format!("{} │ ", text_filled)
} else {
text_filled
}
text_filled
}
}

fn preprocess(&self, text: &str, cursor: &mut usize) -> String {
if self.config.tab_width > 0 {
expand_tabs(text, self.config.tab_width, cursor)
} else {
*cursor += text.len();
text.to_string()
return expand_tabs(text, self.config.tab_width, cursor);
}

*cursor += text.len();
text.to_string()
}
}

Expand Down Expand Up @@ -395,7 +395,7 @@ impl<'a> Printer for InteractivePrinter<'a> {
return Ok(());
}
};
highlighter.highlight(line.as_ref(), self.syntax_set)
highlighter.highlight(&line, self.syntax_set)
};

if out_of_range {
Expand All @@ -420,8 +420,7 @@ impl<'a> Printer for InteractivePrinter<'a> {
let decorations = self
.decorations
.iter()
.map(|d| d.generate(line_number, false, self))
.collect::<Vec<_>>();
.map(|d| d.generate(line_number, false, self));

for deco in decorations {
write!(handle, "{} ", deco.text)?;
Expand All @@ -435,7 +434,7 @@ impl<'a> Printer for InteractivePrinter<'a> {
let colored_output = self.config.colored_output;
let italics = self.config.use_italic_text;

for &(style, region) in regions.iter() {
for &(style, region) in &regions {
let text = &*self.preprocess(region, &mut cursor_total);
let text_trimmed = text.trim_end_matches(|c| c == '\r' || c == '\n');
write!(
Expand Down Expand Up @@ -472,7 +471,7 @@ impl<'a> Printer for InteractivePrinter<'a> {
writeln!(handle)?;
}
} else {
for &(style, region) in regions.iter() {
for &(style, region) in &regions {
let ansi_iterator = AnsiCodeIterator::new(region);
let mut ansi_prefix: String = String::new();
for chunk in ansi_iterator {
Expand Down
8 changes: 3 additions & 5 deletions src/syntax_mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl<'a> SyntaxMapping<'a> {
.unwrap();
}

for glob in [
for glob in &[
"**/systemd/**/*.conf",
"**/systemd/**/*.example",
"*.automount",
Expand All @@ -100,9 +100,7 @@ impl<'a> SyntaxMapping<'a> {
"*.swap",
"*.target",
"*.timer",
]
.iter()
{
] {
mapping.insert(glob, MappingTarget::MapTo("INI")).unwrap();
}

Expand Down Expand Up @@ -153,7 +151,7 @@ impl<'a> SyntaxMapping<'a> {
}

pub(crate) fn get_syntax_for(&self, path: impl AsRef<Path>) -> Option<MappingTarget<'a>> {
let candidate = Candidate::new(path.as_ref());
let candidate = Candidate::new(&path);
let candidate_filename = path.as_ref().file_name().map(Candidate::new);
for (ref glob, ref syntax) in self.mappings.iter().rev() {
if glob.is_match_candidate(&candidate)
Expand Down