Added new logic (suggested in starship#5196 (comment))

The new `detect_env_vars` now requires either SSH_ONLY to be false or the
environment variable SSH_CONNECTION to be set, so that is will be used

docs/config/README.md

@@ -2202,8 +2202,8 @@ The `hostname` module shows the system hostname.
 
 ::: warning
 
-The `detect_env_vars` is only recognized, if `ssh_only` is set to `false`.
-You can still check for a ssh connection, add `SSH_CONNECTION` to `detect_env_vars`.
+The `detect_env_vars` is only used, if `ssh_only` is set to `false` or there
+is an environment variable `SSH_CONNECTION` set.
 
 ### Options
 
@@ -2248,7 +2248,7 @@ disabled = false
 
 [hostname]
 ssh_only = false
-detect_env_vars = ['!tmux', 'SSH_CONNECTION']
+detect_env_vars = ['!TMUX', 'SSH_CONNECTION']
 disabled = false
 ```
 

src/context.rs

@@ -235,20 +235,27 @@ impl<'a> Context<'a> {
         disabled == Some(true)
     }
 
-    /// Returns true when no negated environment variable is defined in `env_vars`
-    /// or none of the negated variables is set in the environment.
-    fn has_no_negative_env_var(&self, env_vars: &'a [&'a str]) -> bool {
-        !env_vars
+    /// Returns true when a negated environment variable is defined in `env_vars` and is present
+    fn has_negated_env_var(&self, env_vars: &'a [&'a str]) -> bool {
+        env_vars
             .iter()
             .any(|env_var| env_var.starts_with('!') && self.get_env(&env_var[1..]).is_some())
     }
 
     /// Returns true if 'detect_env_vars' is empty,
     /// or if at least one environment variable is set and no negated environment variable is set
+    /// or if there are only negated environment variable provided (wich are not set)
     pub fn detect_env_vars(&'a self, env_vars: &'a [&'a str]) -> bool {
-        env_vars.is_empty()
-            || ((env_vars.iter().any(|e| self.get_env(e).is_some()))
-                && self.has_no_negative_env_var(env_vars))
+        if env_vars.is_empty() {
+            true
+        } else if self.has_negated_env_var(env_vars) {
+            false
+        } else {
+            env_vars.iter().all(|env_var| env_var.starts_with('!'))
+                || env_vars
+                    .iter()
+                    .any(|env_var| self.get_env(env_var).is_some())
+        }
     }
 
     // returns a new ScanDir struct with reference to current dir_files of context

src/modules/hostname.rs

@@ -16,10 +16,10 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
     let config: HostnameConfig = HostnameConfig::try_load(module.config);
 
     let ssh_connection = context.get_env("SSH_CONNECTION");
-    if config.ssh_only {
-        ssh_connection.as_ref()?;
-    // check for environment variables
-    } else if !(context.detect_env_vars(&config.detect_env_vars)) {
+
+    if !((!config.ssh_only || ssh_connection.is_some())
+        && context.detect_env_vars(&config.detect_env_vars))
+    {
         return None;
     }
 
@@ -100,103 +100,81 @@ mod tests {
     }
 
     #[test]
-    fn ssh_only_false_with_matching_env_var() {
+    fn ssh_only_false_with_empty_detect_env_vars() {
         let hostname = get_hostname!();
         let actual = ModuleRenderer::new("hostname")
             .config(toml::toml! {
                 [hostname]
                 ssh_only = false
                 trim_at = ""
-                detect_env_vars = ["FORCE_HOSTNAME"]
+                detect_env_vars = []
             })
-            .env("FORCE_HOSTNAME", "true")
             .collect();
 
         let expected = Some(format!("{} in ", style().paint(hostname)));
         assert_eq!(expected, actual);
     }
 
     #[test]
-    fn ssh_only_false_without_matching_env_var() {
+    fn ssh_only_false_with_matching_negated_env_var() {
         let actual = ModuleRenderer::new("hostname")
             .config(toml::toml! {
                 [hostname]
                 ssh_only = false
                 trim_at = ""
-                detect_env_vars = ["FORCE_HOSTNAME"]
+                detect_env_vars = ["!NEGATED"]
             })
+            .env("NEGATED", "true")
             .collect();
         let expected = None;
 
         assert_eq!(expected, actual);
     }
 
     #[test]
-    fn ssh_only_false_and_in_ssh_connection_with_matching_negated_env() {
-        let actual = ModuleRenderer::new("hostname")
-            .config(toml::toml! {
-                [hostname]
-                ssh_only = false
-                trim_at = ""
-                detect_env_vars = ["SSH_CONNECTION", "!TMUX"]
-            })
-            .env("SSH_CONNECTION", "something")
-            .env("TMUX", "true")
-            .collect();
-        let expected = None;
-
-        assert_eq!(expected, actual);
-    }
-
-    #[test]
-    fn ssh_only_false_without_matching_negated_env() {
+    fn ssh_only_false_with_only_negated_env_vars() {
         let hostname = get_hostname!();
         let actual = ModuleRenderer::new("hostname")
             .config(toml::toml! {
                 [hostname]
                 ssh_only = false
                 trim_at = ""
-                detect_env_vars = ["FORCE_HOSTNAME", "!TMUX"]
+                detect_env_vars = ["!NEGATED_ONE", "!NEGATED_TWO", "!NEGATED_THREE"]
             })
-            .env("FORCE_HOSTNAME", "something")
             .collect();
-        let expected = Some(format!("{} in ", style().paint(hostname)));
 
+        let expected = Some(format!("{} in ", style().paint(hostname)));
         assert_eq!(expected, actual);
     }
 
     #[test]
-    fn ssh_only_false_and_in_ssh_connection_without_matching_negated_env() {
+    fn ssh_only_false_with_matching_env_var() {
         let hostname = get_hostname!();
         let actual = ModuleRenderer::new("hostname")
             .config(toml::toml! {
                 [hostname]
                 ssh_only = false
                 trim_at = ""
-                detect_env_vars = ["SSH_CONNECTION", "!TMUX"]
+                detect_env_vars = ["FORCE_HOSTNAME"]
             })
-            .env("SSH_CONNECTION", "something")
+            .env("FORCE_HOSTNAME", "true")
             .collect();
-        let expected = Some(format!(
-            "{} in ",
-            style().paint("🌐 ".to_owned() + hostname.as_str())
-        ));
 
+        let expected = Some(format!("{} in ", style().paint(hostname)));
         assert_eq!(expected, actual);
     }
 
     #[test]
-    fn ssh_only_false_no_ssh() {
-        let hostname = get_hostname!();
+    fn ssh_only_false_without_matching_env_vars() {
         let actual = ModuleRenderer::new("hostname")
             .config(toml::toml! {
                 [hostname]
                 ssh_only = false
                 trim_at = ""
+                detect_env_vars = ["FORCE_HOSTNAME", "!NEGATED"]
             })
             .collect();
-        let expected = Some(format!("{} in ", style().paint(hostname)));
-        println!("{}", expected.as_ref().unwrap());
+        let expected = None;
 
         assert_eq!(expected, actual);
     }