env: move electric variables to impl module

Electric variables are largely an implementation detail, and a following
commit wants to add a circular dependency between the electric var
impl and environment_impl.  Specifically, "ElectricVar" will know
about "EnvScopedImpl". Move both impls into a common impl module,
to make this dependency less surprising.

src/env/environment.rs

@@ -1,9 +1,6 @@
-use super::{
-    ElectricVar,
-    environment_impl::{
-        EnvMutex, EnvMutexGuard, EnvScopedImpl, EnvStackImpl, ModResult, UVAR_SCOPE_IS_GLOBAL,
-        colon_split, uvars,
-    },
+use super::r#impl::environment::{
+    EnvMutex, EnvMutexGuard, EnvScopedImpl, EnvStackImpl, ModResult, UVAR_SCOPE_IS_GLOBAL,
+    colon_split, uvars,
 };
 use crate::{
     abbrs::{Abbreviation, Position, abbrs_get_set},
@@ -11,6 +8,7 @@
     env::{
         EnvMode, EnvSetMode, EnvVar, Statuses,
         config_paths::{ConfigPaths, PREFIX},
+        r#impl::is_electric_var,
     },
     env_dispatch::{VarChangeMilieu, env_dispatch_init, env_dispatch_var_change},
     event::Event,
@@ -584,13 +582,13 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool
         // PORTING: That assumption appears to be wrong https://github.com/rust-lang/rust/blob/2ceed0b6cb9e9866225d7cfcfcbb4a62db047163/library/std/src/sys/unix/os.rs#L584C30-L584C30
         // it appears they allow names starting with =, but do not turn malformed lines
         // into the variable name with an empty value
-        if ElectricVar::for_name(&key).is_none() {
+        if !is_electric_var(&key) && {
             // fish_user_paths should not be exported; attempting to re-import it from
             // a value we previously (due to user error) exported will cause impossibly
             // difficult to debug PATH problems.
-            if key != "fish_user_paths" {
-                vars.set(&key, global_exported_mode, vec![val.clone()]);
-            }
+            key != "fish_user_paths"
+        } {
+            vars.set(&key, global_exported_mode, vec![val.clone()]);
         }
         inherited_vars.insert(key, val);
     }

src/env/impl/environment.rs

@@ -1,6 +1,6 @@
+use super::var::{ELECTRIC_VARIABLES, ElectricVar, is_read_only};
 use crate::env::{
-    ELECTRIC_VARIABLES, ElectricVar, EnvMode, EnvSetMode, EnvStackSetResult, EnvVar, EnvVarFlags,
-    PATH_ARRAY_SEP, Statuses, VarTable, is_read_only,
+    EnvMode, EnvSetMode, EnvStackSetResult, EnvVar, EnvVarFlags, PATH_ARRAY_SEP, Statuses, VarTable,
 };
 use crate::env_universal_common::EnvUniversal;
 use crate::flog::flog;

src/env/impl/mod.rs

@@ -0,0 +1,4 @@
+pub(super) mod environment;
+pub(super) mod var;
+
+pub use var::{is_electric_var, is_read_only};

src/env/impl/var.rs

@@ -0,0 +1,76 @@
+use crate::env::FISH_TERMINAL_COLOR_THEME_VAR;
+use fish_common::assert_sorted_by_name;
+use fish_widestring::{L, wstr};
+
+pub fn is_electric_var(name: &wstr) -> bool {
+    ElectricVar::for_name(name).is_some()
+}
+
+/// Check if a variable may not be set using the set command.
+pub fn is_read_only(name: &wstr) -> bool {
+    ElectricVar::for_name(name).is_some_and(|var| var.readonly())
+}
+
+mod electric {
+    pub(super) const READONLY: u8 = 1 << 0; // May not be modified by the user.
+    pub(super) const COMPUTED: u8 = 1 << 1; // Value is dynamically computed.
+    pub(super) const EXPORTS: u8 = 1 << 2; // Exported to child processes.
+    pub(super) type ElectricVarFlags = u8;
+}
+
+pub(super) struct ElectricVar {
+    pub(super) name: &'static wstr,
+    flags: electric::ElectricVarFlags,
+}
+
+impl ElectricVar {
+    const fn new(name: &'static wstr, flags: electric::ElectricVarFlags) -> Self {
+        Self { name, flags }
+    }
+}
+
+impl ElectricVar {
+    /// Return the ElectricVar with the given name, if any
+    pub(super) fn for_name(name: &wstr) -> Option<&'static ElectricVar> {
+        match ELECTRIC_VARIABLES.binary_search_by(|ev| ev.name.cmp(name)) {
+            Ok(idx) => Some(&ELECTRIC_VARIABLES[idx]),
+            Err(_) => None,
+        }
+    }
+
+    pub(super) fn readonly(&self) -> bool {
+        self.flags & electric::READONLY != 0
+    }
+
+    pub(super) fn computed(&self) -> bool {
+        self.flags & electric::COMPUTED != 0
+    }
+
+    pub(super) fn exports(&self) -> bool {
+        self.flags & electric::EXPORTS != 0
+    }
+}
+
+// Keep sorted alphabetically
+pub(super) const ELECTRIC_VARIABLES: &[ElectricVar] = {
+    use electric::{COMPUTED, EXPORTS, READONLY};
+    let v = ElectricVar::new;
+    &[
+        v(L!("FISH_VERSION"), READONLY),
+        v(L!("PWD"), READONLY | COMPUTED | EXPORTS),
+        v(L!("SHLVL"), READONLY | EXPORTS),
+        v(L!("_"), READONLY),
+        v(L!("fish_kill_signal"), READONLY | COMPUTED),
+        v(L!("fish_killring"), READONLY | COMPUTED),
+        v(L!("fish_pid"), READONLY),
+        v(FISH_TERMINAL_COLOR_THEME_VAR, READONLY),
+        v(L!("history"), READONLY | COMPUTED),
+        v(L!("hostname"), READONLY),
+        v(L!("pipestatus"), READONLY | COMPUTED),
+        v(L!("status"), READONLY | COMPUTED),
+        v(L!("status_generation"), READONLY | COMPUTED),
+        v(L!("umask"), COMPUTED),
+        v(L!("version"), READONLY),
+    ]
+};
+assert_sorted_by_name!(ELECTRIC_VARIABLES);

src/env/mod.rs

@@ -1,12 +1,14 @@
 pub mod config_paths;
 mod environment;
-mod environment_impl;
+mod r#impl;
 mod var;
 
 pub use environment::*;
+pub use r#impl::is_read_only;
+pub use var::*;
+
 use fish_widestring::ToCString;
 use std::sync::{Mutex, atomic::AtomicUsize};
-pub use var::*;
 
 /// Limit `read` to 1 GiB (bytes, not wide chars) by default. This can be overridden with the
 /// `fish_read_limit` variable.

src/env/var.rs

@@ -1,6 +1,6 @@
+use crate::env::r#impl::is_read_only;
 use crate::signal::RawSignal;
 use bitflags::bitflags;
-use fish_common::assert_sorted_by_name;
 use fish_wcstringutil::join_strings;
 use fish_widestring::{L, WString, wstr};
 use libc::c_int;
@@ -249,81 +249,8 @@ pub fn flags_for(name: &wstr) -> EnvVarFlags {
 
 pub type VarTable = HashMap<WString, EnvVar>;
 
-mod electric {
-    pub(super) const READONLY: u8 = 1 << 0; // May not be modified by the user.
-    pub(super) const COMPUTED: u8 = 1 << 1; // Value is dynamically computed.
-    pub(super) const EXPORTS: u8 = 1 << 2; // Exported to child processes.
-    pub(super) type ElectricVarFlags = u8;
-}
-
-pub struct ElectricVar {
-    pub name: &'static wstr,
-    flags: electric::ElectricVarFlags,
-}
-
-impl ElectricVar {
-    const fn new(name: &'static wstr, flags: electric::ElectricVarFlags) -> Self {
-        Self { name, flags }
-    }
-}
-
 pub const FISH_TERMINAL_COLOR_THEME_VAR: &wstr = L!("fish_terminal_color_theme");
 
-// Keep sorted alphabetically
-pub const ELECTRIC_VARIABLES: &[ElectricVar] = {
-    use electric::{COMPUTED, EXPORTS, READONLY};
-    let v = ElectricVar::new;
-    &[
-        v(L!("FISH_VERSION"), READONLY),
-        v(L!("PWD"), READONLY | COMPUTED | EXPORTS),
-        v(L!("SHLVL"), READONLY | EXPORTS),
-        v(L!("_"), READONLY),
-        v(L!("fish_kill_signal"), READONLY | COMPUTED),
-        v(L!("fish_killring"), READONLY | COMPUTED),
-        v(L!("fish_pid"), READONLY),
-        v(FISH_TERMINAL_COLOR_THEME_VAR, READONLY),
-        v(L!("history"), READONLY | COMPUTED),
-        v(L!("hostname"), READONLY),
-        v(L!("pipestatus"), READONLY | COMPUTED),
-        v(L!("status"), READONLY | COMPUTED),
-        v(L!("status_generation"), READONLY | COMPUTED),
-        v(L!("umask"), COMPUTED),
-        v(L!("version"), READONLY),
-    ]
-};
-assert_sorted_by_name!(ELECTRIC_VARIABLES);
-
-impl ElectricVar {
-    /// Return the ElectricVar with the given name, if any
-    pub fn for_name(name: &wstr) -> Option<&'static ElectricVar> {
-        match ELECTRIC_VARIABLES.binary_search_by(|ev| ev.name.cmp(name)) {
-            Ok(idx) => Some(&ELECTRIC_VARIABLES[idx]),
-            Err(_) => None,
-        }
-    }
-
-    pub fn readonly(&self) -> bool {
-        self.flags & electric::READONLY != 0
-    }
-
-    pub fn computed(&self) -> bool {
-        self.flags & electric::COMPUTED != 0
-    }
-
-    pub fn exports(&self) -> bool {
-        self.flags & electric::EXPORTS != 0
-    }
-}
-
-/// Check if a variable may not be set using the set command.
-pub fn is_read_only(name: &wstr) -> bool {
-    if let Some(ev) = ElectricVar::for_name(name) {
-        ev.flags & electric::READONLY != 0
-    } else {
-        false
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::{EnvMode, EnvVar, EnvVarFlags};