Repository: dbrgn/tealdeer
Pull request #276 (2022-06-16 19:22:51)

Allow overriding cache directory through config
Based on #212. Thanks @hgaiser for the initial work!

- Config: Add `cache_dir` to `[directories]` section
- Deprecate old `TEALDEER_CACHE_DIR` env variable
- Introduce path source for `custom_pages_dir` as well
- Add integration test
- Update docs

Implementation notes:

- Logic to handle legacy env var, config override and default cache dir selection is now in the `Config::try_from(RawConfig)` impl
- Logic to determine `custom_pages_dir` is moved into `Config::try_from(RawConfig)` as well for consistency

The logic to determine which cache directory is used is now in `config.rs`, when converting the `RawConfig` to `Config`. I think this is the place where the config file and external factors (defaults, env vars, etc) can be taken into account to determine a final config.

dbrgn: @niklasmohrin sorry for missing #266, I wanted to get PRs unblocked starting at the oldest, and didn't yet see that you already submitted a proposed solution (I skimmed over it since it was still marked as draft)... 🙁

I think I chose a different approach than you did. I determine the final path in `Config::try_from(RawConfig)`. While implementing, I felt that this seems to be a clean solution, because once you get a valid config, you have a final non-nullable config path.

Validation of that path is being done by the cache itself: If the cache is instantiated with a path, it tries to ensure that the path exists. If it doesn't, it is created. If creation fails, the program is aborted.

I'd be interested in your opinion on that.
dbrgn: @niklasmohrin everything should be updated and all your feedback should be addressed!
dbrgn: > Nice! Some more comments, a handful of them are suggestions about removing code comments - I think that the code following those comments is just as expressive (if not more) than the comment while being more precise. I think the comments here add noise and don't provide any benefit. (and, also, all the textbook arguments like "comments aren't run in tests", "comments duplicate knowledge", ...)
> 
> There are more such comments around the codebase (and I would, in most cases, like them gone; not today, but with every little refactor :D). If I recall correctly, we disagreed over this before, haven't we? What did we settle on back then?

Hehe, I know that discussion from other projects 🙂 It comes down to a matter of style, but I think there are good reasons for it. I really like to structure code into blocks that do some kind of action. For example, when creating a file in a directory:

```rust
fn myfunction() {
    check_if_dir_exists();
    check_if_dir_is_writable();
    write_file();
    set_file_permissions();
}
```

In the example above, the three function calls are only 1 line each and self-explanatory. However, when code get bigger, sometimes you need to fully read through the block to understand what it does. With trivial code, that's simple. But if it's a bit more complex, it takes longer.

So I follow a code style where these blocks (potentially spanning multiple lines of code) are preceded with a line comment that explains what the following code does. For example:

```rust
fn myfunction() {
    // Ensure that we can write the directory
    check_if_dir_exists();
    check_if_dir_is_writable();

    // Directory is writable! Actually write the file
    write_file();
    set_file_permissions();
}
```

What I like about this style is that it's really simple to skim over an implementation by just looking at the block headers. Sometimes, blocks are small and self-explanatory, but I usually add these comments anyways for consistency. The idea is not to just state what the next line is doing, but to guide the reader through the code, and - if it makes sense - to also explain the reason why certain things are the way they are.

As a concrete example. This code:

![screenshot-20220928-000532](https://user-images.githubusercontent.com/105168/192646085-05fe93e8-41a3-43cb-81cb-f7fac1f8ac29.png)

...can be skimmed over easily, especially with syntax highlighting. To my mind, I read it like this:

![screenshot-20220928-000649](https://user-images.githubusercontent.com/105168/192646203-6053a29d-f3fa-45ea-8d1a-b8326f768616.png)

...which, I would argue, is much easier to quickly understand than this:

![screenshot-20220928-000729](https://user-images.githubusercontent.com/105168/192646307-6424cfce-b12e-4b7d-9c7d-5e8228037c9f.png)

Note that the last example is _not_ unreadable. I think it's fairly easy to read. But it takes more time and brainpower to process than the version where each block is summarized.

I do kind of feel strongly about this, so I would not want to take away comments, as long as this "block-summary-comment" style is consistent within a function. I don't expect contributors to do this as well, of course, as long as functions remain small and easy to read. (Once functions do get more complex, I think it's valuable to either split them up into multiple functions, or to use comments in a more structured way.)
niklasmohrin: If you say you have a strong opinion about this, I can live with it, although my opinion isn't changed. Here is how I see it:

---

Firstly, I can see that this is useful for reading complex functions, but to me it's unclear where to draw the line. You agreed at the "style config" comment, but I think that
```rust
        // Extract archive into pages dir
        archive
            .extract(&self.pages_dir())
            .context("Could not unpack compressed data")?;
```

and this snippet from main:

```rust
// Initialize logger
init_log();

// Parse arguments
let mut args = Args::parse();
```
look "just as bad" to me. (tbh, I think that all of `Cache::update` is fine without these heading comments).

I find that having a block summarized as just exactly what is already said in the block is just more noisy, so I would first change to something like this:
![grafik](https://user-images.githubusercontent.com/47574893/192863260-b2f1e6bb-9e5c-4c73-97d4-73e017c5a9d7.png)

Now, I too find this a bit confusing. For example, after the renamed args, we have another `if args.something` but that does logic rather than cleanup and one could think that it still belongs to the block before because there is no new block heading (so the _absence_ of a heading confused me). While adding the heading back would help with that slip-up, I think it is not the correct approach. What really happened is that our `main` function just doesn't stick to one level of abstraction and we are trying to patch this up by bundling code into blocks which are then on the same abstraction level. For simple code that is correctly abstracted, this leads to `/* init log */ init_log();`. So what we should aim for is:
![grafik](https://user-images.githubusercontent.com/47574893/192865558-26a88754-2ca7-4386-b144-24e298cb6a0c.png)

I guess what I am trying to say is: I can understand the "summarizing the block" part, but I don't fully agree with the "make usage of block comments consistent within function". To me, it is a symptom of violating the single-level-of-abstraction rule. And yes, it is somewhat eased with helping comments, but in reality we should just try harder to find better words for what the code should do. And maybe, these comments are just a marker for a refactor later, because we are trying to be productive now. But that's it for me.

---

Again, at the end of the day we want to get a feature shipped here and I wouldn't want to have this blocked over a fight about comments (they literally do nothing :D). I am curious how you feel about this though
dbrgn: Great, thanks for the review. All issues should have been addressed.

Regarding the comments: Yes, absolutely, often the clarity of the code can be improved with refactoring or better naming. I'm not saying that shouldn't be done, on the contrary 🙂 And if a method is simple enough, then comments aren't needed. Once it crosses a certain complexity, I simply like adding block comments (in the style of headlines) consistently. Even if this means that sometimes very short blocks have a comment as well. But it looks more consistent.

Any other last feedback? I plan to merge this and prepare the 1.5.1 release by the end of the week.

Patch:
diff --git a/docs/src/config_directories.md b/docs/src/config_directories.md
index d894e90..b194dbb 100644
--- a/docs/src/config_directories.md
+++ b/docs/src/config_directories.md
@@ -2,6 +2,19 @@
 
 This section allows overriding some directory paths.
 
+## `cache_dir`
+
+Override the cache directory. Remember to use an absolute path. Variable
+expansion will not be performed on the path. If the directory does not yet
+exist, it will be created.
+
+    [directories]
+    cache_dir = "/home/myuser/.tealdeer-cache/"
+
+If no `cache_dir` is specified, tealdeer will fall back to a location that
+follows OS conventions. On Linux, it will usually be at `~/.cache/tealdeer/`.
+Use `tldr --show-paths` to show the path that is being used.
+
 ## `custom_pages_dir`
 
 Set the directory to be used to look up [custom
diff --git a/src/cache.rs b/src/cache.rs
index 66b1ef7..57b1a30 100644
--- a/src/cache.rs
+++ b/src/cache.rs
@@ -8,23 +8,20 @@ use std::{
 };
 
 use anyhow::{ensure, Context, Result};
-use app_dirs::{get_app_root, AppDataType};
 use log::debug;
 use reqwest::{blocking::Client, Proxy};
 use walkdir::{DirEntry, WalkDir};
 use zip::ZipArchive;
 
-use crate::types::{PathSource, PlatformType};
-
-static CACHE_DIR_ENV_VAR: &str = "TEALDEER_CACHE_DIR";
+use crate::types::PlatformType;
 
 pub static TLDR_PAGES_DIR: &str = "tldr-pages";
 static TLDR_OLD_PAGES_DIR: &str = "tldr-master";
 
 #[derive(Debug)]
 pub struct Cache {
-    url: String,
     platform: PlatformType,
+    cache_dir: PathBuf,
 }
 
 #[derive(Debug)]
@@ -54,13 +51,13 @@ impl PageLookupResult {
     pub fn reader(&self) -> Result<BufReader<Box<dyn Read>>> {
         // Open page file
         let page_file = File::open(&self.page_path)
-            .with_context(|| format!("Could not open page file at {:?}", self.page_path))?;
+            .with_context(|| format!("Could not open page file at {}", self.page_path.display()))?;
 
         // Open patch file
         let patch_file_opt = match &self.patch_path {
             Some(path) => Some(
                 File::open(path)
-                    .with_context(|| format!("Could not open patch file at {:?}", path))?,
+                    .with_context(|| format!("Could not open patch file at {}", path.display()))?,
             ),
             None => None,
         };
@@ -89,53 +86,57 @@ pub enum CacheFreshness {
 }
 
 impl Cache {
-    pub fn new<S>(url: S, platform: PlatformType) -> Self
+    pub fn new<P>(platform: PlatformType, cache_dir: P) -> Self
     where
-        S: Into<String>,
+        P: Into<PathBuf>,
     {
         Self {
-            url: url.into(),
             platform,
+            cache_dir: cache_dir.into(),
         }
     }
 
-    /// Return the path to the cache directory.
-    pub fn get_cache_dir() -> Result<(PathBuf, PathSource)> {
-        // Allow overriding the cache directory by setting the env variable.
-        if let Ok(value) = env::var(CACHE_DIR_ENV_VAR) {
-            let path = PathBuf::from(value);
-            let (path_exists, path_is_dir) = path
-                .metadata()
-                .map_or((false, false), |md| (true, md.is_dir()));
-            ensure!(
-                !path_exists || path_is_dir,
-                "Path specified by ${} is not a directory",
-                CACHE_DIR_ENV_VAR
+    pub fn cache_dir(&self) -> &Path {
+        &self.cache_dir
+    }
+
+    /// Make sure that the cache directory exists and is a directory.
+    /// If necessary, create the directory.
+    fn ensure_cache_dir_exists(&self) -> Result<()> {
+        // Check whether `cache_dir` exists and is a directory
+        let (cache_dir_exists, cache_dir_is_dir) = self
+            .cache_dir
+            .metadata()
+            .map_or((false, false), |md| (true, md.is_dir()));
+        ensure!(
+            !cache_dir_exists || cache_dir_is_dir,
+            "Cache directory path `{}` is not a directory",
+            self.cache_dir.display(),
+        );
+
+        if !cache_dir_exists {
+            // If missing, try to create the complete directory path
+            fs::create_dir_all(&self.cache_dir).with_context(|| {
+                format!(
+                    "Cache directory path `{}` cannot be created",
+                    self.cache_dir.display(),
+                )
+            })?;
+            eprintln!(
+                "Successfully created cache directory path `{}`.",
+                self.cache_dir.display(),
             );
-            if !path_exists {
-                // Try to create the complete directory path.
-                fs::create_dir_all(&path).with_context(|| {
-                    format!(
-                        "Directory path specified by ${} cannot be created",
-                        CACHE_DIR_ENV_VAR
-                    )
-                })?;
-                eprintln!(
-                    "Successfully created cache directory path `{}`.",
-                    path.to_str().unwrap()
-                );
-            }
-            return Ok((path, PathSource::EnvVar));
-        };
+        }
 
-        // Otherwise, fall back to user cache directory.
-        let dirs = get_app_root(AppDataType::UserCache, &crate::APP_INFO)
-            .context("Could not determine user cache directory")?;
-        Ok((dirs, PathSource::OsConvention))
+        Ok(())
     }
 
-    /// Download the archive
-    fn download(&self) -> Result<Vec<u8>> {
+    fn pages_dir(&self) -> PathBuf {
+        self.cache_dir.join(TLDR_PAGES_DIR)
+    }
+
+    /// Download the archive from the specified URL.
+    fn download(archive_url: &str) -> Result<Vec<u8>> {
         let mut builder = Client::builder();
         if let Ok(ref host) = env::var("HTTP_PROXY") {
             if let Ok(proxy) = Proxy::http(host) {
@@ -151,65 +152,58 @@ impl Cache {
             .build()
             .context("Could not instantiate HTTP client")?;
         let mut resp = client
-            .get(&self.url)
+            .get(archive_url)
             .send()?
             .error_for_status()
-            .with_context(|| format!("Could not download tldr pages from {}", &self.url))?;
+            .with_context(|| format!("Could not download tldr pages from {}", archive_url))?;
         let mut buf: Vec<u8> = vec![];
         let bytes_downloaded = resp.copy_to(&mut buf)?;
         debug!("{} bytes downloaded", bytes_downloaded);
         Ok(buf)
     }
 
-    /// Update the pages cache.
-    pub fn update(&self) -> Result<()> {
+    /// Update the pages cache from the specified URL.
+    pub fn update(&self, archive_url: &str) -> Result<()> {
+        self.ensure_cache_dir_exists()?;
+
         // First, download the compressed data
-        let bytes: Vec<u8> = self.download()?;
+        let bytes: Vec<u8> = Self::download(archive_url)?;
 
         // Decompress the response body into an `Archive`
         let mut archive = ZipArchive::new(Cursor::new(bytes))
             .context("Could not decompress downloaded ZIP archive")?;
 
-        // Determine paths
-        let (cache_dir, _) = Self::get_cache_dir()?;
-        let pages_dir = cache_dir.join(TLDR_PAGES_DIR);
-
-        // Make sure that cache directory exists
-        debug!("Ensure cache directory {:?} exists", &cache_dir);
-        fs::create_dir_all(&cache_dir).context("Could not create cache directory")?;
-
         // Clear cache directory
         // Note: This is not the best solution. Ideally we would download the
         // archive to a temporary directory and then swap the two directories.
         // But renaming a directory doesn't work across filesystems and Rust
         // does not yet offer a recursive directory copying function. So for
         // now, we'll use this approach.
-        Self::clear().context("Could not clear the cache directory")?;
+        self.clear()
+            .context("Could not clear the cache directory")?;
 
-        // Extract archive
+        // Extract archive into pages dir
         archive
-            .extract(&pages_dir)
+            .extract(&self.pages_dir())
             .context("Could not unpack compressed data")?;
 
         Ok(())
     }
 
     /// Return the duration since the cache directory was last modified.
-    pub fn last_update() -> Option<Duration> {
-        if let Ok((cache_dir, _)) = Self::get_cache_dir() {
-            if let Ok(metadata) = fs::metadata(cache_dir.join(TLDR_PAGES_DIR)) {
-                if let Ok(mtime) = metadata.modified() {
-                    let now = SystemTime::now();
-                    return now.duration_since(mtime).ok();
-                };
+    pub fn last_update(&self) -> Option<Duration> {
+        if let Ok(metadata) = fs::metadata(self.pages_dir()) {
+            if let Ok(mtime) = metadata.modified() {
+                let now = SystemTime::now();
+                return now.duration_since(mtime).ok();
             };
         };
         None
     }
 
     /// Return the freshness of the cache (fresh, stale or missing).
-    pub fn freshness() -> CacheFreshness {
-        match Cache::last_update() {
+    pub fn freshness(&self) -> CacheFreshness {
+        match self.last_update() {
             Some(ago) if ago > crate::config::MAX_CACHE_AGE => CacheFreshness::Stale(ago),
             Some(_) => CacheFreshness::Fresh,
             None => CacheFreshness::Missing,
@@ -230,13 +224,13 @@ impl Cache {
     /// Check for pages for a given platform in one of the given languages.
     fn find_page_for_platform(
         page_name: &str,
-        cache_dir: &Path,
+        pages_dir: &Path,
         platform: &str,
         language_dirs: &[String],
     ) -> Option<PathBuf> {
         language_dirs
             .iter()
-            .map(|lang_dir| cache_dir.join(lang_dir).join(platform).join(page_name))
+            .map(|lang_dir| pages_dir.join(lang_dir).join(platform).join(page_name))
             .find(|path| path.exists() && path.is_file())
     }
 
@@ -258,15 +252,8 @@ impl Cache {
         let patch_filename = format!("{}.patch", name);
         let custom_filename = format!("{}.page", name);
 
-        // Get cache dir
-        let cache_dir = match Self::get_cache_dir() {
-            Ok((cache_dir, _)) => cache_dir.join(TLDR_PAGES_DIR),
-            Err(e) => {
-                log::error!("Could not get cache directory: {}", e);
-                return None;
-            }
-        };
-
+        // Determine directory paths
+        let pages_dir = self.pages_dir();
         let lang_dirs: Vec<String> = languages
             .iter()
             .map(|lang| {
@@ -291,21 +278,20 @@ impl Cache {
         // Try to find a platform specific path next, append custom patch to it.
         let platform_dir = self.get_platform_dir();
         if let Some(page) =
-            Self::find_page_for_platform(&page_filename, &cache_dir, platform_dir, &lang_dirs)
+            Self::find_page_for_platform(&page_filename, &pages_dir, platform_dir, &lang_dirs)
         {
             return Some(PageLookupResult::with_page(page).with_optional_patch(patch_path));
         }
 
         // Did not find platform specific results, fall back to "common"
-        Self::find_page_for_platform(&page_filename, &cache_dir, "common", &lang_dirs)
+        Self::find_page_for_platform(&page_filename, &pages_dir, "common", &lang_dirs)
             .map(|page| PageLookupResult::with_page(page).with_optional_patch(patch_path))
     }
 
     /// Return the available pages.
-    pub fn list_pages(&self, custom_pages_dir: Option<&Path>) -> Result<Vec<String>> {
+    pub fn list_pages(&self, custom_pages_dir: Option<&Path>) -> Vec<String> {
         // Determine platforms directory and platform
-        let (cache_dir, _) = Self::get_cache_dir()?;
-        let platforms_dir = cache_dir.join(TLDR_PAGES_DIR).join("pages");
+        let platforms_dir = self.pages_dir().join("pages");
         let platform_dir = self.get_platform_dir();
 
         // Closure that allows the WalkDir instance to traverse platform
@@ -367,29 +353,27 @@ impl Cache {
 
         pages.sort();
         pages.dedup();
-        Ok(pages)
+        pages
     }
 
-    /// Delete the cache directory.
-    pub fn clear() -> Result<()> {
-        let (path, _) = Self::get_cache_dir()?;
-
-        // Check preconditions
-        ensure!(
-            path.exists(),
-            "Cache path ({}) does not exist.",
-            path.display(),
-        );
+    /// Delete the cache directory
+    ///
+    /// Returns true if the cache was deleted and false if the cache dir did
+    /// not exist.
+    pub fn clear(&self) -> Result<bool> {
+        if !self.cache_dir.exists() {
+            return Ok(false);
+        }
         ensure!(
-            path.is_dir(),
+            self.cache_dir.is_dir(),
             "Cache path ({}) is not a directory.",
-            path.display()
+            self.cache_dir.display(),
         );
 
         // Delete old tldr-pages cache location as well if present
         // TODO: To be removed in the future
         for pages_dir_name in [TLDR_PAGES_DIR, TLDR_OLD_PAGES_DIR] {
-            let pages_dir = path.join(pages_dir_name);
+            let pages_dir = self.cache_dir.join(pages_dir_name);
 
             if pages_dir.exists() {
                 fs::remove_dir_all(&pages_dir).with_context(|| {
@@ -401,7 +385,7 @@ impl Cache {
             }
         }
 
-        Ok(())
+        Ok(true)
     }
 }
 
diff --git a/src/config.rs b/src/config.rs
index 74ba195..0889271 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -1,11 +1,11 @@
 use std::{
-    env, fs,
+    env, fmt, fs,
     io::{Read, Write},
-    path::PathBuf,
+    path::{Path, PathBuf},
     time::Duration,
 };
 
-use anyhow::{ensure, Context, Result};
+use anyhow::{bail, ensure, Context, Result};
 use app_dirs::{get_app_root, AppDataType};
 use log::debug;
 use serde_derive::{Deserialize, Serialize};
@@ -129,6 +129,18 @@ struct RawStyleConfig {
     pub example_variable: RawStyle,
 }
 
+impl From<RawStyleConfig> for StyleConfig {
+    fn from(raw_style_config: RawStyleConfig) -> Self {
+        Self {
+            command_name: raw_style_config.command_name.into(),
+            description: raw_style_config.description.into(),
+            example_text: raw_style_config.example_text.into(),
+            example_code: raw_style_config.example_code.into(),
+            example_variable: raw_style_config.example_variable.into(),
+        }
+    }
+}
+
 #[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq)]
 struct RawDisplayConfig {
     #[serde(default)]
@@ -137,6 +149,15 @@ struct RawDisplayConfig {
     pub use_pager: bool,
 }
 
+impl From<RawDisplayConfig> for DisplayConfig {
+    fn from(raw_display_config: RawDisplayConfig) -> Self {
+        Self {
+            compact: raw_display_config.compact,
+            use_pager: raw_display_config.use_pager,
+        }
+    }
+}
+
 /// Serde doesn't support default values yet (tracking issue:
 /// <https://github.com/serde-rs/serde/issues/368>), so we need to wrap
 /// `DEFAULT_UPDATE_INTERVAL_HOURS` in a function to be able to use
@@ -162,25 +183,25 @@ impl Default for RawUpdatesConfig {
     }
 }
 
-#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
-struct RawDirectoriesConfig {
-    #[serde(default)]
-    pub custom_pages_dir: Option<PathBuf>,
-}
-
-impl Default for RawDirectoriesConfig {
-    fn default() -> Self {
+impl From<RawUpdatesConfig> for UpdatesConfig {
+    fn from(raw_updates_config: RawUpdatesConfig) -> Self {
         Self {
-            custom_pages_dir: get_app_root(AppDataType::UserData, &crate::APP_INFO)
-                .map(|path| {
-                    // Note: The `join("")` call ensures that there's a trailing slash
-                    path.join("pages").join("")
-                })
-                .ok(),
+            auto_update: raw_updates_config.auto_update,
+            auto_update_interval: Duration::from_secs(
+                raw_updates_config.auto_update_interval_hours * 3600,
+            ),
         }
     }
 }
 
+#[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq)]
+struct RawDirectoriesConfig {
+    #[serde(default)]
+    pub cache_dir: Option<PathBuf>,
+    #[serde(default)]
+    pub custom_pages_dir: Option<PathBuf>,
+}
+
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
 #[serde(default)]
 struct RawConfig {
@@ -237,9 +258,28 @@ pub struct UpdatesConfig {
     pub auto_update_interval: Duration,
 }
 
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct PathWithSource {
+    pub path: PathBuf,
+    pub source: PathSource,
+}
+
+impl PathWithSource {
+    pub fn path(&self) -> &Path {
+        &self.path
+    }
+}
+
+impl fmt::Display for PathWithSource {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{} ({})", self.path.display(), self.source)
+    }
+}
+
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct DirectoriesConfig {
-    pub custom_pages_dir: Option<PathBuf>,
+    pub cache_dir: PathWithSource,
+    pub custom_pages_dir: Option<PathWithSource>,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -250,34 +290,76 @@ pub struct Config {
     pub directories: DirectoriesConfig,
 }
 
-impl From<RawConfig> for Config {
-    fn from(raw_config: RawConfig) -> Self {
-        Self {
-            style: StyleConfig {
-                command_name: raw_config.style.command_name.into(),
-                description: raw_config.style.description.into(),
-                example_text: raw_config.style.example_text.into(),
-                example_code: raw_config.style.example_code.into(),
-                example_variable: raw_config.style.example_variable.into(),
-            },
-            display: DisplayConfig {
-                compact: raw_config.display.compact,
-                use_pager: raw_config.display.use_pager,
-            },
-            updates: UpdatesConfig {
-                auto_update: raw_config.updates.auto_update,
-                auto_update_interval: Duration::from_secs(
-                    raw_config.updates.auto_update_interval_hours * 3600,
-                ),
-            },
-            directories: DirectoriesConfig {
-                custom_pages_dir: raw_config.directories.custom_pages_dir,
-            },
-        }
+impl Config {
+    /// Convert a `RawConfig` to a high-level `Config`.
+    ///
+    /// For this, some values need to be converted to other types and some
+    /// defaults need to be set (sometimes based on env variables).
+    fn from_raw(raw_config: RawConfig) -> Result<Self> {
+        let style = raw_config.style.into();
+        let display = raw_config.display.into();
+        let updates = raw_config.updates.into();
+
+        // Determine directories config. For this, we need to take some
+        // additional factory into account, like env variables, or the
+        // user config.
+        let cache_dir_env_var = "TEALDEER_CACHE_DIR";
+        let cache_dir = if let Ok(env_var) = env::var(cache_dir_env_var) {
+            // For backwards compatibility reasons, the cache directory can be
+            // overridden using an env variable. This is deprecated and will be
+            // phased out in the future.
+            eprintln!("Warning: The ${} env variable is deprecated, use the `cache_dir` option in the config file instead.", cache_dir_env_var);
+            PathWithSource {
+                path: PathBuf::from(env_var),
+                source: PathSource::EnvVar,
+            }
+        } else if let Some(config_value) = raw_config.directories.cache_dir {
+            // If the user explicitly configured a cache directory, use that.
+            PathWithSource {
+                path: config_value,
+                source: PathSource::ConfigFile,
+            }
+        } else if let Ok(default_dir) = get_app_root(AppDataType::UserCache, &crate::APP_INFO) {
+            // Otherwise, fall back to the default user cache directory.
+            PathWithSource {
+                path: default_dir,
+                source: PathSource::OsConvention,
+            }
+        } else {
+            // If everything fails, give up
+            bail!("Could not determine user cache directory");
+        };
+        let custom_pages_dir = raw_config
+            .directories
+            .custom_pages_dir
+            .map(|path| PathWithSource {
+                path,
+                source: PathSource::OsConvention,
+            })
+            .or_else(|| {
+                get_app_root(AppDataType::UserData, &crate::APP_INFO)
+                    .map(|path| {
+                        // Note: The `join("")` call ensures that there's a trailing slash
+                        PathWithSource {
+                            path: path.join("pages").join(""),
+                            source: PathSource::ConfigFile,
+                        }
+                    })
+                    .ok()
+            });
+        let directories = DirectoriesConfig {
+            cache_dir,
+            custom_pages_dir,
+        };
+
+        Ok(Self {
+            style,
+            display,
+            updates,
+            directories,
+        })
     }
-}
 
-impl Config {
     pub fn load(enable_styles: bool) -> Result<Self> {
         debug!("Loading config");
 
@@ -301,7 +383,7 @@ impl Config {
         };
 
         // Convert to config
-        let mut config = Self::from(raw_config);
+        let mut config = Self::from_raw(raw_config).context("Could not process raw config")?;
 
         // Potentially override styles
         if !enable_styles {
diff --git a/src/main.rs b/src/main.rs
index 6752358..ffacae2 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -43,7 +43,7 @@ mod utils;
 use crate::{
     cache::{Cache, CacheFreshness, PageLookupResult, TLDR_PAGES_DIR},
     cli::Args,
-    config::{get_config_dir, get_config_path, make_default_config, Config},
+    config::{get_config_dir, get_config_path, make_default_config, Config, PathWithSource},
     extensions::Dedup,
     output::print_page,
     types::{ColorOptions, PlatformType},
@@ -59,11 +59,13 @@ const ARCHIVE_URL: &str = "https://tldr.sh/assets/tldr.zip";
 
 /// The cache should be updated if it was explicitly requested,
 /// or if an automatic update is due and allowed.
-fn should_update_cache(args: &Args, config: &Config) -> bool {
+fn should_update_cache(cache: &Cache, args: &Args, config: &Config) -> bool {
     args.update
         || (!args.no_auto_update
             && config.updates.auto_update
-            && Cache::last_update().map_or(true, |ago| ago >= config.updates.auto_update_interval))
+            && cache
+                .last_update()
+                .map_or(true, |ago| ago >= config.updates.auto_update_interval))
 }
 
 #[derive(PartialEq)]
@@ -73,8 +75,8 @@ enum CheckCacheResult {
 }
 
 /// Check the cache for freshness. If it's stale or missing, show a warning.
-fn check_cache(args: &Args, enable_styles: bool) -> CheckCacheResult {
-    match Cache::freshness() {
+fn check_cache(cache: &Cache, args: &Args, enable_styles: bool) -> CheckCacheResult {
+    match cache.freshness() {
         CacheFreshness::Fresh => CheckCacheResult::CacheFound,
         CacheFreshness::Stale(_) if args.quiet => CheckCacheResult::CacheFound,
         CacheFreshness::Stale(age) => {
@@ -109,19 +111,27 @@ fn check_cache(args: &Args, enable_styles: bool) -> CheckCacheResult {
 }
 
 /// Clear the cache
-fn clear_cache(quietly: bool, enable_styles: bool) {
-    Cache::clear().unwrap_or_else(|e| {
+fn clear_cache(cache: &Cache, quietly: bool, enable_styles: bool) {
+    let cache_dir_found = cache.clear().unwrap_or_else(|e| {
         print_error(enable_styles, &e.context("Could not clear cache"));
         process::exit(1);
     });
     if !quietly {
-        eprintln!("Successfully deleted cache.");
+        let cache_dir = cache.cache_dir().display();
+        if cache_dir_found {
+            eprintln!("Successfully cleared cache at `{}`.", cache_dir);
+        } else {
+            eprintln!(
+                "Cache directory not found at `{}`, nothing to do.",
+                cache_dir
+            );
+        }
     }
 }
 
 /// Update the cache
 fn update_cache(cache: &Cache, quietly: bool, enable_styles: bool) {
-    cache.update().unwrap_or_else(|e| {
+    cache.update(ARCHIVE_URL).unwrap_or_else(|e| {
         print_error(enable_styles, &e.context("Could not update cache"));
         process::exit(1);
     });
@@ -157,35 +167,19 @@ fn show_paths(config: &Config) {
     );
     let config_path = get_config_path().map_or_else(
         |e| format!("[Error: {}]", e),
-        |(path, _)| path.to_str().unwrap_or("[Invalid]").to_string(),
-    );
-    let cache_dir = Cache::get_cache_dir().map_or_else(
-        |e| format!("[Error: {}]", e),
-        |(mut path, source)| {
-            path.push(""); // Trailing path separator
-            match path.to_str() {
-                Some(path) => format!("{} ({})", path, source),
-                None => "[Invalid]".to_string(),
-            }
-        },
-    );
-    let pages_dir = Cache::get_cache_dir().map_or_else(
-        |e| format!("[Error: {}]", e),
-        |(mut path, _)| {
-            path.push(TLDR_PAGES_DIR);
-            path.push(""); // Trailing path separator
-            path.into_os_string()
-                .into_string()
-                .unwrap_or_else(|_| "[Invalid]".to_string())
-        },
-    );
-    let custom_pages_dir = config.directories.custom_pages_dir.as_deref().map_or_else(
-        || "[None]".to_string(),
-        |path| {
-            path.to_str()
-                .map_or_else(|| "[Invalid]".to_string(), ToString::to_string)
-        },
+        |(path, _)| path.display().to_string(),
     );
+    let cache_dir = config.directories.cache_dir.to_string();
+    let pages_dir = {
+        let mut path = config.directories.cache_dir.path.clone();
+        path.push(TLDR_PAGES_DIR);
+        path.push(""); // Trailing path separator
+        path.display().to_string()
+    };
+    let custom_pages_dir = match config.directories.custom_pages_dir {
+        Some(ref path_with_source) => path_with_source.to_string(),
+        None => "[None]".to_string(),
+    };
     println!("Config dir:       {}", config_dir);
     println!("Config path:      {}", config_path);
     println!("Cache dir:        {}", cache_dir);
@@ -338,16 +332,16 @@ fn main() {
         };
     }
 
-    // Initialize cache
-    let cache = Cache::new(ARCHIVE_URL, platform);
+    // Instantiate cache. This will not yet create the cache directory!
+    let cache = Cache::new(platform, &config.directories.cache_dir.path);
 
     // Clear cache, pass through
     if args.clear_cache {
-        clear_cache(args.quiet, enable_styles);
+        clear_cache(&cache, args.quiet, enable_styles);
     }
 
     // Cache update, pass through
-    let cache_updated = if should_update_cache(&args, &config) {
+    let cache_updated = if should_update_cache(&cache, &args, &config) {
         update_cache(&cache, args.quiet, enable_styles);
         true
     } else {
@@ -357,23 +351,19 @@ fn main() {
     // Check cache presence and freshness
     if !cache_updated
         && (args.list || !args.command.is_empty())
-        && check_cache(&args, enable_styles) == CheckCacheResult::CacheMissing
+        && check_cache(&cache, &args, enable_styles) == CheckCacheResult::CacheMissing
     {
         process::exit(1);
     }
 
     // List cached commands and exit
     if args.list {
-        // Get list of pages
-        let pages = cache
-            .list_pages(config.directories.custom_pages_dir.as_deref())
-            .unwrap_or_else(|e| {
-                print_error(enable_styles, &e.context("Could not get list of pages"));
-                process::exit(1);
-            });
-
-        // Print pages
-        println!("{}", pages.join("\n"));
+        let custom_pages_dir = config
+            .directories
+            .custom_pages_dir
+            .as_ref()
+            .map(PathWithSource::path);
+        println!("{}", cache.list_pages(custom_pages_dir).join("\n"));
         process::exit(0);
     }
 
@@ -393,7 +383,11 @@ fn main() {
         if let Some(lookup_result) = cache.find_page(
             &command,
             &languages,
-            config.directories.custom_pages_dir.as_deref(),
+            config
+                .directories
+                .custom_pages_dir
+                .as_ref()
+                .map(PathWithSource::path),
         ) {
             if let Err(ref e) =
                 print_page(&lookup_result, args.raw, enable_styles, args.pager, &config)
diff --git a/src/types.rs b/src/types.rs
index 683a81a..f3318a9 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -185,16 +185,14 @@ impl LineType {
 }
 
 /// The reason why a certain path (e.g. config path or cache dir) was chosen.
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Debug, PartialEq, Eq, Copy, Clone)]
 pub enum PathSource {
     /// OS convention (e.g. XDG on Linux)
     OsConvention,
     /// Env variable (TEALDEER_*)
     EnvVar,
-
-    #[allow(dead_code)] // Waiting for Pull Request #141
-    /// Config file variable
-    ConfigVar,
+    /// Config file
+    ConfigFile,
 }
 
 impl fmt::Display for PathSource {
@@ -205,7 +203,7 @@ impl fmt::Display for PathSource {
             match self {
                 Self::OsConvention => "OS convention",
                 Self::EnvVar => "env variable",
-                Self::ConfigVar => "config file variable",
+                Self::ConfigFile => "config file",
             }
         )
     }


Test patch:
diff --git a/tests/lib.rs b/tests/lib.rs
index 6e66764..230b2f2 100644
--- a/tests/lib.rs
+++ b/tests/lib.rs
@@ -10,9 +10,9 @@ use std::{
 use assert_cmd::prelude::*;
 use predicates::{
     boolean::PredicateBooleanExt,
-    prelude::predicate::str::{contains, diff, is_empty},
+    prelude::predicate::str::{contains, diff, is_empty, is_match},
 };
-use tempfile::{Builder, TempDir};
+use tempfile::{Builder as TempfileBuilder, TempDir};
 
 // TODO: Should be 'cache::CACHE_DIR_ENV_VAR'. This requires to have a library crate for the logic.
 static CACHE_DIR_ENV_VAR: &str = "TEALDEER_CACHE_DIR";
@@ -31,13 +31,22 @@ struct TestEnv {
 impl TestEnv {
     fn new() -> Self {
         TestEnv {
-            cache_dir: Builder::new().prefix(".tldr.test.cache").tempdir().unwrap(),
-            config_dir: Builder::new().prefix(".tldr.test.conf").tempdir().unwrap(),
-            custom_pages_dir: Builder::new()
+            cache_dir: TempfileBuilder::new()
+                .prefix(".tldr.test.cache")
+                .tempdir()
+                .unwrap(),
+            config_dir: TempfileBuilder::new()
+                .prefix(".tldr.test.conf")
+                .tempdir()
+                .unwrap(),
+            custom_pages_dir: TempfileBuilder::new()
                 .prefix(".tldr.test.custom-pages")
                 .tempdir()
                 .unwrap(),
-            input_dir: Builder::new().prefix(".tldr.test.input").tempdir().unwrap(),
+            input_dir: TempfileBuilder::new()
+                .prefix(".tldr.test.input")
+                .tempdir()
+                .unwrap(),
             default_features: true,
             features: vec![],
         }
@@ -263,9 +272,53 @@ fn test_cache_location_not_a_directory() {
         .assert()
         .failure()
         .stderr(contains(format!(
-            "Path specified by ${} is not a directory",
-            CACHE_DIR_ENV_VAR
-        )));
+            "Cache directory path `{}` is not a directory",
+            internal_file.display(),
+        )))
+        .stderr(contains(
+            "Warning: The $TEALDEER_CACHE_DIR env variable is deprecated",
+        ));
+}
+
+#[test]
+fn test_cache_location_source() {
+    let testenv = TestEnv::new();
+    let default_cache_dir = testenv.cache_dir.path();
+    let tmp_cache_dir = TempfileBuilder::new()
+        .prefix(".tldr.test.cache_dir")
+        .tempdir()
+        .unwrap();
+
+    // Source: Default (OS convention)
+    let mut command = testenv.command();
+    command.env_remove(CACHE_DIR_ENV_VAR);
+    command
+        .arg("--show-paths")
+        .assert()
+        .success()
+        .stdout(is_match("\nCache dir:        [^(]* \\(OS convention\\)\n").unwrap());
+
+    // Source: Config variable
+    let mut command = testenv.command();
+    command.env_remove(CACHE_DIR_ENV_VAR);
+    testenv.write_config(format!(
+        "[directories]\ncache_dir = '{}'",
+        tmp_cache_dir.path().to_str().unwrap(),
+    ));
+    command
+        .arg("--show-paths")
+        .assert()
+        .success()
+        .stdout(is_match("\nCache dir:        [^(]* \\(config file\\)\n").unwrap());
+
+    // Source: Env var
+    let mut command = testenv.command();
+    command.env(CACHE_DIR_ENV_VAR, default_cache_dir.to_str().unwrap());
+    command
+        .arg("--show-paths")
+        .assert()
+        .success()
+        .stdout(is_match("\nCache dir:        [^(]* \\(env variable\\)\n").unwrap());
 }
 
 #[test]