Merge pull request #2393 from miles170/issue-2392-cache-clear-target-dir

Fix `bat cache --clear` not clearing the `--target` dir if specified

CHANGELOG.md

@@ -7,6 +7,8 @@
 
 ## Bugfixes
 
+- Fix `bat cache --clear` not clearing the `--target` dir if specified. See #2393 (@miles170)
+
 ## Other
 
 - Various bash completion improvements, see #2310 (@scop)

src/bin/bat/assets.rs

@@ -1,31 +1,24 @@
-use std::borrow::Cow;
 use std::fs;
 use std::io;
+use std::path::Path;
+use std::path::PathBuf;
 
 use clap::crate_version;
 
-use crate::directories::PROJECT_DIRS;
-
 use bat::assets::HighlightingAssets;
 use bat::assets_metadata::AssetsMetadata;
 use bat::error::*;
 
-pub fn config_dir() -> Cow<'static, str> {
+pub fn clear_assets(cache_dir: &Path) {
-    PROJECT_DIRS.config_dir().to_string_lossy()
+    clear_asset(cache_dir.join("themes.bin"), "theme set cache");
+    clear_asset(cache_dir.join("syntaxes.bin"), "syntax set cache");
+    clear_asset(cache_dir.join("metadata.yaml"), "metadata file");
 }
 
-pub fn cache_dir() -> Cow<'static, str> {
+pub fn assets_from_cache_or_binary(
-    PROJECT_DIRS.cache_dir().to_string_lossy()
+    use_custom_assets: bool,
-}
+    cache_dir: &Path,
-
+) -> Result<HighlightingAssets> {
-pub fn clear_assets() {
-    clear_asset("themes.bin", "theme set cache");
-    clear_asset("syntaxes.bin", "syntax set cache");
-    clear_asset("metadata.yaml", "metadata file");
-}
-
-pub fn assets_from_cache_or_binary(use_custom_assets: bool) -> Result<HighlightingAssets> {
-    let cache_dir = PROJECT_DIRS.cache_dir();
     if let Some(metadata) = AssetsMetadata::load_from_folder(cache_dir)? {
         if !metadata.is_compatible_with(crate_version!()) {
             return Err(format!(
@@ -50,9 +43,8 @@ pub fn assets_from_cache_or_binary(use_custom_assets: bool) -> Result<Highlighti
     Ok(custom_assets.unwrap_or_else(HighlightingAssets::from_binary))
 }
 
-fn clear_asset(filename: &str, description: &str) {
+fn clear_asset(path: PathBuf, description: &str) {
     print!("Clearing {} ... ", description);
-    let path = PROJECT_DIRS.cache_dir().join(filename);
     match fs::remove_file(&path) {
         Err(err) if err.kind() == io::ErrorKind::NotFound => {
             println!("skipped (not present)");

src/bin/bat/main.rs

@@ -25,7 +25,7 @@ use crate::{
 #[cfg(feature = "bugreport")]
 use crate::config::system_config_file;
 
-use assets::{assets_from_cache_or_binary, cache_dir, clear_assets, config_dir};
+use assets::{assets_from_cache_or_binary, clear_assets};
 use directories::PROJECT_DIRS;
 use globset::GlobMatcher;
 
@@ -41,33 +41,38 @@ use bat::{
 const THEME_PREVIEW_DATA: &[u8] = include_bytes!("../../../assets/theme_preview.rs");
 
 #[cfg(feature = "build-assets")]
-fn build_assets(matches: &clap::ArgMatches) -> Result<()> {
+fn build_assets(matches: &clap::ArgMatches, config_dir: &Path, cache_dir: &Path) -> Result<()> {
     let source_dir = matches
         .get_one::<String>("source")
         .map(Path::new)
-        .unwrap_or_else(|| PROJECT_DIRS.config_dir());
+        .unwrap_or_else(|| config_dir);
-    let target_dir = matches
-        .get_one::<String>("target")
-        .map(Path::new)
-        .unwrap_or_else(|| PROJECT_DIRS.cache_dir());
 
     bat::assets::build(
         source_dir,
         !matches.get_flag("blank"),
         matches.get_flag("acknowledgements"),
-        target_dir,
+        cache_dir,
         clap::crate_version!(),
     )
 }
 
-fn run_cache_subcommand(matches: &clap::ArgMatches) -> Result<()> {
+fn run_cache_subcommand(
+    matches: &clap::ArgMatches,
+    #[cfg(feature = "build-assets")] config_dir: &Path,
+    default_cache_dir: &Path,
+) -> Result<()> {
+    let cache_dir = matches
+        .get_one::<String>("target")
+        .map(Path::new)
+        .unwrap_or_else(|| default_cache_dir);
+
     if matches.get_flag("build") {
         #[cfg(feature = "build-assets")]
-        build_assets(matches)?;
+        build_assets(matches, config_dir, cache_dir)?;
         #[cfg(not(feature = "build-assets"))]
         println!("bat has been built without the 'build-assets' feature. The 'cache --build' option is not available.");
     } else if matches.get_flag("clear") {
-        clear_assets();
+        clear_assets(cache_dir);
     }
 
     Ok(())
@@ -86,10 +91,10 @@ fn get_syntax_mapping_to_paths<'a>(
     map
 }
 
-pub fn get_languages(config: &Config) -> Result<String> {
+pub fn get_languages(config: &Config, cache_dir: &Path) -> Result<String> {
     let mut result: String = String::new();
 
-    let assets = assets_from_cache_or_binary(config.use_custom_assets)?;
+    let assets = assets_from_cache_or_binary(config.use_custom_assets, cache_dir)?;
     let mut languages = assets
         .get_syntaxes()?
         .iter()
@@ -181,8 +186,8 @@ fn theme_preview_file<'a>() -> Input<'a> {
     Input::from_reader(Box::new(BufReader::new(THEME_PREVIEW_DATA)))
 }
 
-pub fn list_themes(cfg: &Config) -> Result<()> {
+pub fn list_themes(cfg: &Config, config_dir: &Path, cache_dir: &Path) -> Result<()> {
-    let assets = assets_from_cache_or_binary(cfg.use_custom_assets)?;
+    let assets = assets_from_cache_or_binary(cfg.use_custom_assets, cache_dir)?;
     let mut config = cfg.clone();
     let mut style = HashSet::new();
     style.insert(StyleComponent::Plain);
@@ -211,7 +216,7 @@ pub fn list_themes(cfg: &Config) -> Result<()> {
             and are added to the cache with `bat cache --build`. \
             For more information, see:\n\n  \
             https://github.com/sharkdp/bat#adding-new-themes",
-            PROJECT_DIRS.config_dir().join("themes").to_string_lossy()
+            config_dir.join("themes").to_string_lossy()
         )?;
     } else {
         for theme in assets.themes() {
@@ -222,21 +227,21 @@ pub fn list_themes(cfg: &Config) -> Result<()> {
     Ok(())
 }
 
-fn run_controller(inputs: Vec<Input>, config: &Config) -> Result<bool> {
+fn run_controller(inputs: Vec<Input>, config: &Config, cache_dir: &Path) -> Result<bool> {
-    let assets = assets_from_cache_or_binary(config.use_custom_assets)?;
+    let assets = assets_from_cache_or_binary(config.use_custom_assets, cache_dir)?;
     let controller = Controller::new(config, &assets);
     controller.run(inputs)
 }
 
 #[cfg(feature = "bugreport")]
-fn invoke_bugreport(app: &App) {
+fn invoke_bugreport(app: &App, cache_dir: &Path) {
     use bugreport::{bugreport, collector::*, format::Markdown};
     let pager = bat::config::get_pager_executable(
         app.matches.get_one::<String>("pager").map(|s| s.as_str()),
     )
     .unwrap_or_else(|| "less".to_owned()); // FIXME: Avoid non-canonical path to "less".
 
-    let mut custom_assets_metadata = PROJECT_DIRS.cache_dir().to_path_buf();
+    let mut custom_assets_metadata = cache_dir.to_path_buf();
     custom_assets_metadata.push("metadata.yaml");
 
     let mut report = bugreport!()
@@ -268,10 +273,7 @@ fn invoke_bugreport(app: &App) {
             "Custom assets metadata",
             custom_assets_metadata,
         ))
-        .info(DirectoryEntries::new(
+        .info(DirectoryEntries::new("Custom assets", cache_dir))
-            "Custom assets",
-            PROJECT_DIRS.cache_dir(),
-        ))
         .info(CompileTimeInformation::default());
 
     #[cfg(feature = "paging")]
@@ -290,10 +292,12 @@ fn invoke_bugreport(app: &App) {
 /// `Ok(false)` if any intermediate errors occurred (were printed).
 fn run() -> Result<bool> {
     let app = App::new()?;
+    let config_dir = PROJECT_DIRS.config_dir();
+    let cache_dir = PROJECT_DIRS.cache_dir();
 
     if app.matches.get_flag("diagnostic") {
         #[cfg(feature = "bugreport")]
-        invoke_bugreport(&app);
+        invoke_bugreport(&app, cache_dir);
         #[cfg(not(feature = "bugreport"))]
         println!("bat has been built without the 'bugreport' feature. The '--diagnostic' option is not available.");
         return Ok(true);
@@ -305,13 +309,18 @@ fn run() -> Result<bool> {
             // arguments for subcommand 'cache' are not mandatory.
             // If there are non-zero arguments, execute the subcommand cache, else, open the file cache.
             if cache_matches.args_present() {
-                run_cache_subcommand(cache_matches)?;
+                run_cache_subcommand(
+                    cache_matches,
+                    #[cfg(feature = "build-assets")]
+                    config_dir,
+                    cache_dir,
+                )?;
                 Ok(true)
             } else {
                 let inputs = vec![Input::ordinary_file("cache")];
                 let config = app.config(&inputs)?;
 
-                run_controller(inputs, &config)
+                run_controller(inputs, &config, cache_dir)
             }
         }
         _ => {
@@ -319,16 +328,16 @@ fn run() -> Result<bool> {
             let config = app.config(&inputs)?;
 
             if app.matches.get_flag("list-languages") {
-                let languages: String = get_languages(&config)?;
+                let languages: String = get_languages(&config, cache_dir)?;
                 let inputs: Vec<Input> = vec![Input::from_reader(Box::new(languages.as_bytes()))];
                 let plain_config = Config {
                     style_components: StyleComponents::new(StyleComponent::Plain.components(false)),
                     paging_mode: PagingMode::QuitIfOneScreen,
                     ..Default::default()
                 };
-                run_controller(inputs, &plain_config)
+                run_controller(inputs, &plain_config, cache_dir)
             } else if app.matches.get_flag("list-themes") {
-                list_themes(&config)?;
+                list_themes(&config, config_dir, cache_dir)?;
                 Ok(true)
             } else if app.matches.get_flag("config-file") {
                 println!("{}", config_file().to_string_lossy());
@@ -337,16 +346,16 @@ fn run() -> Result<bool> {
                 generate_config_file()?;
                 Ok(true)
             } else if app.matches.get_flag("config-dir") {
-                writeln!(io::stdout(), "{}", config_dir())?;
+                writeln!(io::stdout(), "{}", config_dir.to_string_lossy())?;
                 Ok(true)
             } else if app.matches.get_flag("cache-dir") {
-                writeln!(io::stdout(), "{}", cache_dir())?;
+                writeln!(io::stdout(), "{}", cache_dir.to_string_lossy())?;
                 Ok(true)
             } else if app.matches.get_flag("acknowledgements") {
                 writeln!(io::stdout(), "{}", bat::assets::get_acknowledgements())?;
                 Ok(true)
             } else {
-                run_controller(inputs, &config)
+                run_controller(inputs, &config, cache_dir)
             }
         }
     }

tests/integration_tests.rs

@@ -892,11 +892,8 @@ fn config_read_arguments_from_file() {
         .stdout(predicate::eq("dummy-pager-from-config\n").normalize());
 }
 
-// Ignore this test for now as `bat cache --clear` only targets the default cache dir.
-// `bat cache --clear` must clear the `--target` dir for this test to pass.
 #[cfg(unix)]
 #[test]
-#[ignore]
 fn cache_clear() {
     let src_dir = "cache_source";
     let tmp_dir = tempdir().expect("can create temporary directory");