From c93bd7b705919966c90f5879bb8aaf4d22cf058c Mon Sep 17 00:00:00 2001
From: Jan Katins <jasc@gmx.net>
Date: Sat, 26 Sep 2020 09:35:41 +0200
Subject: [PATCH] fix: actually disable per default disabled modules (#1677)

The default `disabled: true` is actually only available within the module (when the config struct is used and not the user toml) but not all (the hg_branch) modules checked it there again.

Document this in all places and add the check (+ test) to the hg_branch module.
---
 src/modules/hg_branch.rs    | 28 ++++++++++++++++++++++++++++
 src/modules/kubernetes.rs   |  3 +++
 src/modules/memory_usage.rs |  2 ++
 src/modules/shlvl.rs        |  3 +++
 src/modules/time.rs         |  3 +++
 5 files changed, 39 insertions(+)

diff --git a/src/modules/hg_branch.rs b/src/modules/hg_branch.rs
index 7b301945f..7d29d622f 100644
--- a/src/modules/hg_branch.rs
+++ b/src/modules/hg_branch.rs
@@ -18,6 +18,12 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
     let mut module = context.new_module("hg_branch");
     let config: HgBranchConfig = HgBranchConfig::try_load(module.config);
 
+    // As we default to disabled=true, we have to check here after loading our config module,
+    // before it was only checking against whatever is in the config starship.toml
+    if config.disabled {
+        return None;
+    };
+
     // TODO: Once error handling is implemented, warn the user if their config
     // truncation length is nonsensical
     let len = if config.truncation_length <= 0 {
@@ -125,6 +131,24 @@ mod tests {
         repo_dir.close()
     }
 
+    #[test]
+    #[ignore]
+    fn test_hg_disabled_per_default() -> io::Result<()> {
+        let tempdir = fixture_repo(FixtureProvider::HG)?;
+        let repo_dir = tempdir.path();
+        run_hg(&["whatever", "blubber"], &repo_dir)?;
+        expect_hg_branch_with_config(
+            &repo_dir,
+            // no "disabled=false" in config!
+            Some(toml::toml! {
+                [hg_branch]
+                truncation_length = 14
+            }),
+            &[Expect::Empty],
+        )?;
+        tempdir.close()
+    }
+
     #[test]
     #[ignore]
     fn test_hg_get_branch_fails() -> io::Result<()> {
@@ -186,6 +210,7 @@ mod tests {
             Some(toml::toml! {
                 [hg_branch]
                 truncation_length = 14
+                disabled = false
             }),
             &[Expect::BranchName(&"branch-name-10")],
         )?;
@@ -215,6 +240,7 @@ mod tests {
                 symbol = "B "
                 truncation_length = 14
                 truncation_symbol = "%"
+                disabled = false
             }),
             &[
                 Expect::BranchName(&"branch-name-12"),
@@ -247,6 +273,7 @@ mod tests {
             Some(toml::toml! {
                 [hg_branch]
                 style = "underline blue"
+                disabled = false
             }),
             &[
                 Expect::BranchName(&"branch-name-131"),
@@ -267,6 +294,7 @@ mod tests {
             .config(config.unwrap_or_else(|| {
                 toml::toml! {
                     [hg_branch]
+                    disabled = false
                 }
             }))
             .collect();
diff --git a/src/modules/kubernetes.rs b/src/modules/kubernetes.rs
index a10a33aa6..bf687f74d 100644
--- a/src/modules/kubernetes.rs
+++ b/src/modules/kubernetes.rs
@@ -58,6 +58,9 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
 
             let mut module = context.new_module("kubernetes");
             let config: KubernetesConfig = KubernetesConfig::try_load(module.config);
+
+            // As we default to disabled=true, we have to check here after loading our config module,
+            // before it was only checking against whatever is in the config starship.toml
             if config.disabled {
                 return None;
             };
diff --git a/src/modules/memory_usage.rs b/src/modules/memory_usage.rs
index 51fe1b59d..f3041c0ec 100644
--- a/src/modules/memory_usage.rs
+++ b/src/modules/memory_usage.rs
@@ -33,6 +33,8 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
         _ => "%",
     };
 
+    // As we default to disabled=true, we have to check here after loading our config module,
+    // before it was only checking against whatever is in the config starship.toml
     if config.disabled {
         return None;
     }
diff --git a/src/modules/shlvl.rs b/src/modules/shlvl.rs
index 2118c8010..b21f91219 100644
--- a/src/modules/shlvl.rs
+++ b/src/modules/shlvl.rs
@@ -12,6 +12,8 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
     let mut module = context.new_module("shlvl");
     let config: ShLvlConfig = ShLvlConfig::try_load(module.config);
 
+    // As we default to disabled=true, we have to check here after loading our config module,
+    // before it was only checking against whatever is in the config starship.toml
     if config.disabled || shlvl < config.threshold {
         return None;
     }
@@ -45,6 +47,7 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
 
     Some(module)
 }
+
 #[cfg(test)]
 mod tests {
     use ansi_term::{Color, Style};
diff --git a/src/modules/time.rs b/src/modules/time.rs
index f0228b68b..f0ce32db9 100644
--- a/src/modules/time.rs
+++ b/src/modules/time.rs
@@ -8,6 +8,9 @@ use crate::formatter::StringFormatter;
 pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
     let mut module = context.new_module("time");
     let config: TimeConfig = TimeConfig::try_load(module.config);
+
+    // As we default to disabled=true, we have to check here after loading our config module,
+    // before it was only checking against whatever is in the config starship.toml
     if config.disabled {
         return None;
     };