Don't take a HighlightingAssets detour to build assets (#1802)

Move code to build assets to its own file. That results in better modularity and flexibility.

It also allows us to simplify HighlightingAssets a lot, since it will now always
be initialized with a SerializedSyntaxSet.

CHANGELOG.md

@@ -30,6 +30,7 @@
 ## `bat` as a library
 
 - Deprecate `HighlightingAssets::syntaxes()` and `HighlightingAssets::syntax_for_file_name()`. Use `HighlightingAssets::get_syntaxes()` and `HighlightingAssets::get_syntax_for_file_name()` instead. They return a `Result` which is needed for upcoming lazy-loading work to improve startup performance. They also return what `SyntaxSet` the returned `SyntaxReference` belongs to. See #1747, #1755 and #1776 (@Enselic)
+- Remove `HighlightingAssets::from_files` and `HighlightingAssets::save_to_cache`. Instead of calling the former and then the latter you now make a single call to `bat::assets::build`. See #1802 (@Enselic)
 
 
 

src/assets.rs

@@ -18,7 +18,7 @@ use crate::syntax_mapping::{MappingTarget, SyntaxMapping};
 #[derive(Debug)]
 pub struct HighlightingAssets {
     syntax_set_cell: LazyCell<SyntaxSet>,
-    serialized_syntax_set: Option<SerializedSyntaxSet>,
+    serialized_syntax_set: SerializedSyntaxSet,
     theme_set: ThemeSet,
     fallback_theme: Option<&'static str>,
 }
@@ -47,20 +47,9 @@ const IGNORED_SUFFIXES: [&str; 10] = [
 ];
 
 impl HighlightingAssets {
-    fn new(
+    fn new(serialized_syntax_set: SerializedSyntaxSet, theme_set: ThemeSet) -> Self {
-        syntax_set: Option<SyntaxSet>,
-        serialized_syntax_set: Option<SerializedSyntaxSet>,
-        theme_set: ThemeSet,
-    ) -> Self {
-        assert!(syntax_set.is_some() || serialized_syntax_set.is_some());
-
-        let syntax_set_cell = LazyCell::new();
-        if let Some(syntax_set) = syntax_set {
-            syntax_set_cell.fill(syntax_set).expect("can never fail");
-        }
-
         HighlightingAssets {
-            syntax_set_cell,
+            syntax_set_cell: LazyCell::new(),
             serialized_syntax_set,
             theme_set,
             fallback_theme: None,
@@ -71,127 +60,27 @@ impl HighlightingAssets {
         "Monokai Extended"
     }
 
-    #[cfg(feature = "build-assets")]
-    pub fn from_files(source_dir: &Path, include_integrated_assets: bool) -> Result<Self> {
-        let mut theme_set = if include_integrated_assets {
-            get_integrated_themeset()
-        } else {
-            ThemeSet::new()
-        };
-
-        let theme_dir = source_dir.join("themes");
-        if theme_dir.exists() {
-            let res = theme_set.add_from_folder(&theme_dir);
-            if let Err(err) = res {
-                println!(
-                    "Failed to load one or more themes from '{}' (reason: '{}')",
-                    theme_dir.to_string_lossy(),
-                    err,
-                );
-            }
-        } else {
-            println!(
-                "No themes were found in '{}', using the default set",
-                theme_dir.to_string_lossy()
-            );
-        }
-
-        let mut syntax_set_builder = if !include_integrated_assets {
-            let mut builder = syntect::parsing::SyntaxSetBuilder::new();
-            builder.add_plain_text_syntax();
-            builder
-        } else {
-            from_binary::<SyntaxSet>(get_serialized_integrated_syntaxset()).into_builder()
-        };
-
-        let syntax_dir = source_dir.join("syntaxes");
-        if syntax_dir.exists() {
-            syntax_set_builder.add_from_folder(syntax_dir, true)?;
-        } else {
-            println!(
-                "No syntaxes were found in '{}', using the default set.",
-                syntax_dir.to_string_lossy()
-            );
-        }
-
-        if std::env::var("BAT_PRINT_SYNTAX_DEPENDENCIES").is_ok() {
-            // To trigger this code, run:
-            // BAT_PRINT_SYNTAX_DEPENDENCIES=1 cargo run -- cache --build --source assets --blank --target /tmp
-            crate::syntax_dependencies::print_syntax_dependencies(&syntax_set_builder);
-        }
-
-        let syntax_set = syntax_set_builder.build();
-        let missing_contexts = syntax_set.find_unlinked_contexts();
-        if !missing_contexts.is_empty() {
-            println!("Some referenced contexts could not be found!");
-            for context in missing_contexts {
-                println!("- {}", context);
-            }
-        }
-
-        Ok(HighlightingAssets::new(Some(syntax_set), None, theme_set))
-    }
-
     pub fn from_cache(cache_path: &Path) -> Result<Self> {
         Ok(HighlightingAssets::new(
-            None,
+            SerializedSyntaxSet::FromFile(cache_path.join("syntaxes.bin")),
-            Some(SerializedSyntaxSet::FromFile(
-                cache_path.join("syntaxes.bin"),
-            )),
             asset_from_cache(&cache_path.join("themes.bin"), "theme set")?,
         ))
     }
 
     pub fn from_binary() -> Self {
         HighlightingAssets::new(
-            None,
+            SerializedSyntaxSet::FromBinary(get_serialized_integrated_syntaxset()),
-            Some(SerializedSyntaxSet::FromBinary(
-                get_serialized_integrated_syntaxset(),
-            )),
             get_integrated_themeset(),
         )
     }
 
-    #[cfg(feature = "build-assets")]
-    pub fn save_to_cache(&self, target_dir: &Path, current_version: &str) -> Result<()> {
-        let _ = fs::create_dir_all(target_dir);
-        asset_to_cache(
-            self.get_theme_set(),
-            &target_dir.join("themes.bin"),
-            "theme set",
-        )?;
-        asset_to_cache(
-            self.get_syntax_set()?,
-            &target_dir.join("syntaxes.bin"),
-            "syntax set",
-        )?;
-
-        print!(
-            "Writing metadata to folder {} ... ",
-            target_dir.to_string_lossy()
-        );
-        crate::assets_metadata::AssetsMetadata::new(current_version).save_to_folder(target_dir)?;
-        println!("okay");
-
-        Ok(())
-    }
-
     pub fn set_fallback_theme(&mut self, theme: &'static str) {
         self.fallback_theme = Some(theme);
     }
 
     pub(crate) fn get_syntax_set(&self) -> Result<&SyntaxSet> {
-        if !self.syntax_set_cell.filled() {
+        self.syntax_set_cell
-            self.syntax_set_cell.fill(
+            .try_borrow_with(|| self.serialized_syntax_set.deserialize())
-                self.serialized_syntax_set
-                .as_ref()
-                .expect("a dev forgot to setup serialized_syntax_set, please report to https://github.com/sharkdp/bat/issues")
-                .deserialize()?
-             ).unwrap();
-        }
-
-        // It is safe to .unwrap() because we just made sure it was .filled()
-        Ok(self.syntax_set_cell.borrow().unwrap())
     }
 
     /// Use [Self::get_syntaxes] instead
@@ -393,6 +282,9 @@ impl HighlightingAssets {
     }
 }
 
+#[cfg(feature = "build-assets")]
+pub use crate::build_assets::build_assets as build;
+
 /// A SyntaxSet in serialized form, i.e. bincoded and flate2 compressed.
 /// We keep it in this format since we want to load it lazily.
 #[derive(Debug)]
@@ -413,28 +305,14 @@ impl SerializedSyntaxSet {
     }
 }
 
-fn get_serialized_integrated_syntaxset() -> &'static [u8] {
+pub(crate) fn get_serialized_integrated_syntaxset() -> &'static [u8] {
     include_bytes!("../assets/syntaxes.bin")
 }
 
-fn get_integrated_themeset() -> ThemeSet {
+pub(crate) fn get_integrated_themeset() -> ThemeSet {
     from_binary(include_bytes!("../assets/themes.bin"))
 }
 
-#[cfg(feature = "build-assets")]
-fn asset_to_cache<T: serde::Serialize>(asset: &T, path: &Path, description: &str) -> Result<()> {
-    print!("Writing {} to {} ... ", description, path.to_string_lossy());
-    syntect::dumps::dump_to_file(asset, &path).chain_err(|| {
-        format!(
-            "Could not save {} to {}",
-            description,
-            path.to_string_lossy()
-        )
-    })?;
-    println!("okay");
-    Ok(())
-}
-
 fn asset_from_cache<T: serde::de::DeserializeOwned>(path: &Path, description: &str) -> Result<T> {
     let contents = fs::read(path).chain_err(|| {
         format!(

src/bin/bat/main.rs

@@ -50,8 +50,7 @@ fn build_assets(matches: &clap::ArgMatches) -> Result<()> {
 
     let blank = matches.is_present("blank");
 
-    let assets = bat::assets::HighlightingAssets::from_files(source_dir, !blank)?;
+    bat::assets::build(source_dir, !blank, target_dir, clap::crate_version!())
-    assets.save_to_cache(target_dir, clap::crate_version!())
 }
 
 fn run_cache_subcommand(matches: &clap::ArgMatches) -> Result<()> {

src/build_assets.rs

@@ -1,9 +1,14 @@
 use std::collections::HashMap;
+use std::path::Path;
+use syntect::dumps::from_binary;
+use syntect::highlighting::ThemeSet;
 use syntect::parsing::syntax_definition::{
     ContextReference, MatchOperation, MatchPattern, Pattern, SyntaxDefinition,
 };
 use syntect::parsing::{Scope, SyntaxSet, SyntaxSetBuilder};
 
+use crate::error::*;
+
 type SyntaxName = String;
 
 /// Used to look up what dependencies a given [SyntaxDefinition] has
@@ -22,9 +27,86 @@ enum Dependency {
     ByScope(Scope),
 }
 
+pub fn build_assets(
+    source_dir: &Path,
+    include_integrated_assets: bool,
+    target_dir: &Path,
+    current_version: &str,
+) -> Result<()> {
+    let mut theme_set = if include_integrated_assets {
+        crate::assets::get_integrated_themeset()
+    } else {
+        ThemeSet::new()
+    };
+
+    let theme_dir = source_dir.join("themes");
+    if theme_dir.exists() {
+        let res = theme_set.add_from_folder(&theme_dir);
+        if let Err(err) = res {
+            println!(
+                "Failed to load one or more themes from '{}' (reason: '{}')",
+                theme_dir.to_string_lossy(),
+                err,
+            );
+        }
+    } else {
+        println!(
+            "No themes were found in '{}', using the default set",
+            theme_dir.to_string_lossy()
+        );
+    }
+
+    let mut syntax_set_builder = if !include_integrated_assets {
+        let mut builder = syntect::parsing::SyntaxSetBuilder::new();
+        builder.add_plain_text_syntax();
+        builder
+    } else {
+        from_binary::<SyntaxSet>(crate::assets::get_serialized_integrated_syntaxset())
+            .into_builder()
+    };
+
+    let syntax_dir = source_dir.join("syntaxes");
+    if syntax_dir.exists() {
+        syntax_set_builder.add_from_folder(syntax_dir, true)?;
+    } else {
+        println!(
+            "No syntaxes were found in '{}', using the default set.",
+            syntax_dir.to_string_lossy()
+        );
+    }
+
+    if std::env::var("BAT_PRINT_SYNTAX_DEPENDENCIES").is_ok() {
+        // To trigger this code, run:
+        // BAT_PRINT_SYNTAX_DEPENDENCIES=1 cargo run -- cache --build --source assets --blank --target /tmp
+        print_syntax_dependencies(&syntax_set_builder);
+    }
+
+    let syntax_set = syntax_set_builder.build();
+    let missing_contexts = syntax_set.find_unlinked_contexts();
+    if !missing_contexts.is_empty() {
+        println!("Some referenced contexts could not be found!");
+        for context in missing_contexts {
+            println!("- {}", context);
+        }
+    }
+
+    let _ = std::fs::create_dir_all(target_dir);
+    asset_to_cache(&theme_set, &target_dir.join("themes.bin"), "theme set")?;
+    asset_to_cache(&syntax_set, &target_dir.join("syntaxes.bin"), "syntax set")?;
+
+    print!(
+        "Writing metadata to folder {} ... ",
+        target_dir.to_string_lossy()
+    );
+    crate::assets_metadata::AssetsMetadata::new(current_version).save_to_folder(target_dir)?;
+    println!("okay");
+
+    Ok(())
+}
+
 /// Generates independent [SyntaxSet]s after analyzing dependencies between syntaxes
 /// in a [SyntaxSetBuilder], and then prints the reults.
-pub(crate) fn print_syntax_dependencies(syntax_set_builder: &SyntaxSetBuilder) {
+fn print_syntax_dependencies(syntax_set_builder: &SyntaxSetBuilder) {
     println!("Constructing independent SyntaxSets...");
     let independent_syntax_sets = build_independent_syntax_sets(syntax_set_builder);
 
@@ -182,3 +264,16 @@ impl SyntaxSetDependencyBuilder {
         self.syntax_set_builder.build()
     }
 }
+
+fn asset_to_cache<T: serde::Serialize>(asset: &T, path: &Path, description: &str) -> Result<()> {
+    print!("Writing {} to {} ... ", description, path.to_string_lossy());
+    syntect::dumps::dump_to_file(asset, &path).chain_err(|| {
+        format!(
+            "Could not save {} to {}",
+            description,
+            path.to_string_lossy()
+        )
+    })?;
+    println!("okay");
+    Ok(())
+}

src/lib.rs

@@ -23,6 +23,8 @@ mod macros;
 
 pub mod assets;
 pub mod assets_metadata;
+#[cfg(feature = "build-assets")]
+mod build_assets;
 pub mod config;
 pub mod controller;
 mod decorations;
@@ -40,8 +42,6 @@ mod preprocessor;
 mod pretty_printer;
 pub(crate) mod printer;
 pub mod style;
-#[cfg(feature = "build-assets")]
-mod syntax_dependencies;
 pub(crate) mod syntax_mapping;
 mod terminal;
 pub(crate) mod wrapping;