Skip to content

Commit d8cc199

Browse files
EnselicEnselic
committed
Introduce failable version of HighlightingAssets::syntaxes()
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 #1747). This commit deprecates old public syntaxes() and introduces a failable version called get_syntaxes().
1 parent f464b1b commit d8cc199

File tree

5 files changed

+64
-26
lines changed

5 files changed

+64
-26
lines changed

‎src/assets.rs‎

Lines changed: 57 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ impl HighlightingAssets {
129129
let _ = fs::create_dir_all(target_dir);
130130
asset_to_cache(&self.theme_set, &target_dir.join("themes.bin"), "theme set")?;
131131
asset_to_cache(
132-
self.get_syntax_set(),
132+
self.get_syntax_set()?,
133133
&target_dir.join("syntaxes.bin"),
134134
"syntax set",
135135
)?;
@@ -148,12 +148,20 @@ impl HighlightingAssets {
148148
self.fallback_theme = Some(theme);
149149
}
150150

151-
pub(crate) fn get_syntax_set(&self) -> &SyntaxSet {
152-
&self.syntax_set
151+
pub(crate) fn get_syntax_set(&self) -> Result<&SyntaxSet> {
152+
Ok(&self.syntax_set)
153153
}
154154

155+
/// Use [Self::get_syntaxes] instead
156+
#[deprecated]
155157
pub fn syntaxes(&self) -> &[SyntaxReference] {
156-
self.get_syntax_set().syntaxes()
158+
self.get_syntax_set()
159+
.expect("This method is deprecated, use get_syntaxes() instead")
160+
.syntaxes()
161+
}
162+
163+
pub fn get_syntaxes(&self) -> Result<&[SyntaxReference]> {
164+
Ok(self.get_syntax_set()?.syntaxes())
157165
}
158166

159167
pub fn themes(&self) -> impl Iterator<Item = &str> {
@@ -168,9 +176,10 @@ impl HighlightingAssets {
168176
let file_name = file_name.as_ref();
169177
match mapping.get_syntax_for(file_name) {
170178
Some(MappingTarget::MapToUnknown) => None,
171-
Some(MappingTarget::MapTo(syntax_name)) => {
172-
self.get_syntax_set().find_syntax_by_name(syntax_name)
173-
}
179+
Some(MappingTarget::MapTo(syntax_name)) => self
180+
.get_syntax_set()
181+
.ok()
182+
.and_then(|ss| ss.find_syntax_by_name(syntax_name)),
174183
None => self.get_extension_syntax(file_name.as_os_str()),
175184
}
176185
}
@@ -198,7 +207,7 @@ impl HighlightingAssets {
198207
mapping: &SyntaxMapping,
199208
) -> Result<&SyntaxReference> {
200209
if let Some(language) = language {
201-
self.get_syntax_set()
210+
self.get_syntax_set()?
202211
.find_syntax_by_token(language)
203212
.ok_or_else(|| ErrorKind::UnknownSyntax(language.to_owned()).into())
204213
} else {
@@ -231,7 +240,7 @@ impl HighlightingAssets {
231240
}),
232241

233242
Some(MappingTarget::MapTo(syntax_name)) => self
234-
.get_syntax_set()
243+
.get_syntax_set()?
235244
.find_syntax_by_name(syntax_name)
236245
.ok_or_else(|| ErrorKind::UnknownSyntax(syntax_name.to_owned()).into()),
237246

@@ -253,16 +262,20 @@ impl HighlightingAssets {
253262

254263
fn get_extension_syntax(&self, file_name: &OsStr) -> Option<&SyntaxReference> {
255264
self.get_syntax_set()
256-
.find_syntax_by_extension(file_name.to_str().unwrap_or_default())
265+
.ok()
266+
.and_then(|ss| ss.find_syntax_by_extension(file_name.to_str().unwrap_or_default()))
257267
.or_else(|| {
258268
let file_path = Path::new(file_name);
259269
self.get_syntax_set()
260-
.find_syntax_by_extension(
261-
file_path
262-
.extension()
263-
.and_then(|x| x.to_str())
264-
.unwrap_or_default(),
265-
)
270+
.ok()
271+
.and_then(|ss| {
272+
ss.find_syntax_by_extension(
273+
file_path
274+
.extension()
275+
.and_then(|x| x.to_str())
276+
.unwrap_or_default(),
277+
)
278+
})
266279
.or_else(|| {
267280
if let Some(file_str) = file_path.to_str() {
268281
for suffix in IGNORED_SUFFIXES.iter() {
@@ -280,7 +293,11 @@ impl HighlightingAssets {
280293
fn get_first_line_syntax(&self, reader: &mut InputReader) -> Option<&SyntaxReference> {
281294
String::from_utf8(reader.first_line.clone())
282295
.ok()
283-
.and_then(|l| self.get_syntax_set().find_syntax_by_first_line(&l))
296+
.and_then(|l| {
297+
self.get_syntax_set()
298+
.ok()
299+
.and_then(|ss| ss.find_syntax_by_first_line(&l))
300+
})
284301
}
285302
}
286303

@@ -353,7 +370,12 @@ mod tests {
353370

354371
self.assets
355372
.get_syntax(None, &mut opened_input, &self.syntax_mapping)
356-
.unwrap_or_else(|_| self.assets.get_syntax_set().find_syntax_plain_text())
373+
.unwrap_or_else(|_| {
374+
self.assets
375+
.get_syntax_set()
376+
.unwrap()
377+
.find_syntax_plain_text()
378+
})
357379
.name
358380
.clone()
359381
}
@@ -367,7 +389,12 @@ mod tests {
367389

368390
self.assets
369391
.get_syntax(None, &mut opened_input, &self.syntax_mapping)
370-
.unwrap_or_else(|_| self.assets.get_syntax_set().find_syntax_plain_text())
392+
.unwrap_or_else(|_| {
393+
self.assets
394+
.get_syntax_set()
395+
.unwrap()
396+
.find_syntax_plain_text()
397+
})
371398
.name
372399
.clone()
373400
}
@@ -391,7 +418,12 @@ mod tests {
391418

392419
self.assets
393420
.get_syntax(None, &mut opened_input, &self.syntax_mapping)
394-
.unwrap_or_else(|_| self.assets.get_syntax_set().find_syntax_plain_text())
421+
.unwrap_or_else(|_| {
422+
self.assets
423+
.get_syntax_set()
424+
.unwrap()
425+
.find_syntax_plain_text()
426+
})
395427
.name
396428
.clone()
397429
}
@@ -549,7 +581,11 @@ mod tests {
549581
assert_eq!(
550582
test.assets
551583
.get_syntax(None, &mut opened_input, &test.syntax_mapping)
552-
.unwrap_or_else(|_| test.assets.get_syntax_set().find_syntax_plain_text())
584+
.unwrap_or_else(|_| test
585+
.assets
586+
.get_syntax_set()
587+
.unwrap()
588+
.find_syntax_plain_text())
553589
.name,
554590
"SSH Config"
555591
);

‎src/bin/bat/main.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ pub fn get_languages(config: &Config) -> Result<String> {
8282

8383
let assets = assets_from_cache_or_binary()?;
8484
let mut languages = assets
85-
.syntaxes()
85+
.get_syntaxes()?
8686
.iter()
8787
.filter(|syntax| !syntax.hidden && !syntax.file_extensions.is_empty())
8888
.cloned()

‎src/pretty_printer.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,9 @@ impl<'a> PrettyPrinter<'a> {
235235
}
236236

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

241243
/// Pretty-print all specified inputs. This method will "use" all stored inputs.

‎src/printer.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ impl<'a> InteractivePrinter<'a> {
174174
let syntax = match assets.get_syntax(config.language, input, &config.syntax_mapping) {
175175
Ok(syntax) => syntax,
176176
Err(Error(ErrorKind::UndetectedSyntax(_), _)) => {
177-
assets.get_syntax_set().find_syntax_plain_text()
177+
assets.get_syntax_set()?.find_syntax_plain_text()
178178
}
179179
Err(e) => return Err(e),
180180
};
@@ -192,7 +192,7 @@ impl<'a> InteractivePrinter<'a> {
192192
#[cfg(feature = "git")]
193193
line_changes,
194194
highlighter,
195-
syntax_set: assets.get_syntax_set(),
195+
syntax_set: assets.get_syntax_set()?,
196196
background_color_highlight,
197197
})
198198
}

‎tests/no_duplicate_extensions.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ fn no_duplicate_extensions() {
2626

2727
let mut extensions = HashSet::new();
2828

29-
for syntax in assets.syntaxes() {
29+
for syntax in assets.get_syntaxes().unwrap() {
3030
for extension in &syntax.file_extensions {
3131
assert!(
3232
KNOWN_EXCEPTIONS.contains(&extension.as_str()) || extensions.insert(extension),

0 commit comments

Comments
 (0)