Skip to content

Commit

Permalink
Introduce failable version of HighlightingAssets::syntaxes()
Browse files Browse the repository at this point in the history
To enable robust and user-friendly support for lazy-loading, we need variants of
get_syntax_set() and syntaxes() that can fail (see discussion about panics in
sharkdp#1747).

This commit deprecates old public syntaxes() and introduces a failable version
called get_syntaxes().
  • Loading branch information
Enselic committed Jul 27, 2021
1 parent f464b1b commit d8cc199
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 26 deletions.
78 changes: 57 additions & 21 deletions src/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl HighlightingAssets {
let _ = fs::create_dir_all(target_dir);
asset_to_cache(&self.theme_set, &target_dir.join("themes.bin"), "theme set")?;
asset_to_cache(
self.get_syntax_set(),
self.get_syntax_set()?,
&target_dir.join("syntaxes.bin"),
"syntax set",
)?;
Expand All @@ -148,12 +148,20 @@ impl HighlightingAssets {
self.fallback_theme = Some(theme);
}

pub(crate) fn get_syntax_set(&self) -> &SyntaxSet {
&self.syntax_set
pub(crate) fn get_syntax_set(&self) -> Result<&SyntaxSet> {
Ok(&self.syntax_set)
}

/// Use [Self::get_syntaxes] instead
#[deprecated]
pub fn syntaxes(&self) -> &[SyntaxReference] {
self.get_syntax_set().syntaxes()
self.get_syntax_set()
.expect("This method is deprecated, use get_syntaxes() instead")
.syntaxes()
}

pub fn get_syntaxes(&self) -> Result<&[SyntaxReference]> {
Ok(self.get_syntax_set()?.syntaxes())
}

pub fn themes(&self) -> impl Iterator<Item = &str> {
Expand All @@ -168,9 +176,10 @@ impl HighlightingAssets {
let file_name = file_name.as_ref();
match mapping.get_syntax_for(file_name) {
Some(MappingTarget::MapToUnknown) => None,
Some(MappingTarget::MapTo(syntax_name)) => {
self.get_syntax_set().find_syntax_by_name(syntax_name)
}
Some(MappingTarget::MapTo(syntax_name)) => self
.get_syntax_set()
.ok()
.and_then(|ss| ss.find_syntax_by_name(syntax_name)),
None => self.get_extension_syntax(file_name.as_os_str()),
}
}
Expand Down Expand Up @@ -198,7 +207,7 @@ impl HighlightingAssets {
mapping: &SyntaxMapping,
) -> Result<&SyntaxReference> {
if let Some(language) = language {
self.get_syntax_set()
self.get_syntax_set()?
.find_syntax_by_token(language)
.ok_or_else(|| ErrorKind::UnknownSyntax(language.to_owned()).into())
} else {
Expand Down Expand Up @@ -231,7 +240,7 @@ impl HighlightingAssets {
}),

Some(MappingTarget::MapTo(syntax_name)) => self
.get_syntax_set()
.get_syntax_set()?
.find_syntax_by_name(syntax_name)
.ok_or_else(|| ErrorKind::UnknownSyntax(syntax_name.to_owned()).into()),

Expand All @@ -253,16 +262,20 @@ impl HighlightingAssets {

fn get_extension_syntax(&self, file_name: &OsStr) -> Option<&SyntaxReference> {
self.get_syntax_set()
.find_syntax_by_extension(file_name.to_str().unwrap_or_default())
.ok()
.and_then(|ss| ss.find_syntax_by_extension(file_name.to_str().unwrap_or_default()))
.or_else(|| {
let file_path = Path::new(file_name);
self.get_syntax_set()
.find_syntax_by_extension(
file_path
.extension()
.and_then(|x| x.to_str())
.unwrap_or_default(),
)
.ok()
.and_then(|ss| {
ss.find_syntax_by_extension(
file_path
.extension()
.and_then(|x| x.to_str())
.unwrap_or_default(),
)
})
.or_else(|| {
if let Some(file_str) = file_path.to_str() {
for suffix in IGNORED_SUFFIXES.iter() {
Expand All @@ -280,7 +293,11 @@ impl HighlightingAssets {
fn get_first_line_syntax(&self, reader: &mut InputReader) -> Option<&SyntaxReference> {
String::from_utf8(reader.first_line.clone())
.ok()
.and_then(|l| self.get_syntax_set().find_syntax_by_first_line(&l))
.and_then(|l| {
self.get_syntax_set()
.ok()
.and_then(|ss| ss.find_syntax_by_first_line(&l))
})
}
}

Expand Down Expand Up @@ -353,7 +370,12 @@ mod tests {

self.assets
.get_syntax(None, &mut opened_input, &self.syntax_mapping)
.unwrap_or_else(|_| self.assets.get_syntax_set().find_syntax_plain_text())
.unwrap_or_else(|_| {
self.assets
.get_syntax_set()
.unwrap()
.find_syntax_plain_text()
})
.name
.clone()
}
Expand All @@ -367,7 +389,12 @@ mod tests {

self.assets
.get_syntax(None, &mut opened_input, &self.syntax_mapping)
.unwrap_or_else(|_| self.assets.get_syntax_set().find_syntax_plain_text())
.unwrap_or_else(|_| {
self.assets
.get_syntax_set()
.unwrap()
.find_syntax_plain_text()
})
.name
.clone()
}
Expand All @@ -391,7 +418,12 @@ mod tests {

self.assets
.get_syntax(None, &mut opened_input, &self.syntax_mapping)
.unwrap_or_else(|_| self.assets.get_syntax_set().find_syntax_plain_text())
.unwrap_or_else(|_| {
self.assets
.get_syntax_set()
.unwrap()
.find_syntax_plain_text()
})
.name
.clone()
}
Expand Down Expand Up @@ -549,7 +581,11 @@ mod tests {
assert_eq!(
test.assets
.get_syntax(None, &mut opened_input, &test.syntax_mapping)
.unwrap_or_else(|_| test.assets.get_syntax_set().find_syntax_plain_text())
.unwrap_or_else(|_| test
.assets
.get_syntax_set()
.unwrap()
.find_syntax_plain_text())
.name,
"SSH Config"
);
Expand Down
2 changes: 1 addition & 1 deletion src/bin/bat/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub fn get_languages(config: &Config) -> Result<String> {

let assets = assets_from_cache_or_binary()?;
let mut languages = assets
.syntaxes()
.get_syntaxes()?
.iter()
.filter(|syntax| !syntax.hidden && !syntax.file_extensions.is_empty())
.cloned()
Expand Down
4 changes: 3 additions & 1 deletion src/pretty_printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,9 @@ impl<'a> PrettyPrinter<'a> {
}

pub fn syntaxes(&self) -> impl Iterator<Item = &SyntaxReference> {
self.assets.syntaxes().iter()
// We always use assets from the binary, which are guaranteed to always
// be valid, so get_syntaxes() can never fail here
self.assets.get_syntaxes().unwrap().iter()
}

/// Pretty-print all specified inputs. This method will "use" all stored inputs.
Expand Down
4 changes: 2 additions & 2 deletions src/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ impl<'a> InteractivePrinter<'a> {
let syntax = match assets.get_syntax(config.language, input, &config.syntax_mapping) {
Ok(syntax) => syntax,
Err(Error(ErrorKind::UndetectedSyntax(_), _)) => {
assets.get_syntax_set().find_syntax_plain_text()
assets.get_syntax_set()?.find_syntax_plain_text()
}
Err(e) => return Err(e),
};
Expand All @@ -192,7 +192,7 @@ impl<'a> InteractivePrinter<'a> {
#[cfg(feature = "git")]
line_changes,
highlighter,
syntax_set: assets.get_syntax_set(),
syntax_set: assets.get_syntax_set()?,
background_color_highlight,
})
}
Expand Down
2 changes: 1 addition & 1 deletion tests/no_duplicate_extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ fn no_duplicate_extensions() {

let mut extensions = HashSet::new();

for syntax in assets.syntaxes() {
for syntax in assets.get_syntaxes().unwrap() {
for extension in &syntax.file_extensions {
assert!(
KNOWN_EXCEPTIONS.contains(&extension.as_str()) || extensions.insert(extension),
Expand Down

0 comments on commit d8cc199

Please sign in to comment.