refactor

- add `diff.ignoreSubmodules` configuration key with a strong type
- add test assertions to nail most relevant behaviour.

Author: Byron

gix/src/config/tree/sections/diff.rs

@@ -15,6 +15,12 @@ impl Diff {
     .with_note(
         "The limit is actually squared, so 1000 stands for up to 1 million diffs if fuzzy rename tracking is enabled",
     );
+
+    /// The `diff.ignoreSubmodules` key.
+    pub const IGNORE_SUBMODULES: Ignore =
+        Ignore::new_with_validate("ignoreSubmodules", &config::Tree::DIFF, validate::Ignore)
+            .with_note("This setting affects only the submodule status, and thus the repository status in general.");
+
     /// The `diff.renames` key.
     pub const RENAMES: Renames = Renames::new_renames("renames", &config::Tree::DIFF);
 
@@ -45,6 +51,7 @@ impl Section for Diff {
     fn keys(&self) -> &[&dyn Key] {
         &[
             &Self::ALGORITHM,
+            &Self::IGNORE_SUBMODULES,
             &Self::RENAME_LIMIT,
             &Self::RENAMES,
             &Self::DRIVER_COMMAND,
@@ -59,6 +66,9 @@ impl Section for Diff {
 /// The `diff.algorithm` key.
 pub type Algorithm = keys::Any<validate::Algorithm>;
 
+/// The `diff.ignoreSubmodules` key.
+pub type Ignore = keys::Any<validate::Ignore>;
+
 /// The `diff.renames` key.
 pub type Renames = keys::Any<validate::Renames>;
 
@@ -71,12 +81,27 @@ mod algorithm {
     use crate::{
         bstr::BStr,
         config,
-        config::{diff::algorithm::Error, tree::sections::diff::Algorithm},
+        config::{
+            diff::algorithm,
+            key,
+            tree::sections::diff::{Algorithm, Ignore},
+        },
     };
 
+    impl Ignore {
+        /// See if `value` is an actual ignore
+        pub fn try_into_ignore(
+            &'static self,
+            value: Cow<'_, BStr>,
+        ) -> Result<gix_submodule::config::Ignore, key::GenericErrorWithValue> {
+            gix_submodule::config::Ignore::try_from(value.as_ref())
+                .map_err(|()| key::GenericErrorWithValue::from_value(self, value.into_owned()))
+        }
+    }
+
     impl Algorithm {
         /// Derive the diff algorithm identified by `name`, case-insensitively.
-        pub fn try_into_algorithm(&self, name: Cow<'_, BStr>) -> Result<gix_diff::blob::Algorithm, Error> {
+        pub fn try_into_algorithm(&self, name: Cow<'_, BStr>) -> Result<gix_diff::blob::Algorithm, algorithm::Error> {
             let algo = if name.eq_ignore_ascii_case(b"myers") || name.eq_ignore_ascii_case(b"default") {
                 gix_diff::blob::Algorithm::Myers
             } else if name.eq_ignore_ascii_case(b"minimal") {
@@ -88,7 +113,7 @@ mod algorithm {
                     name: name.into_owned(),
                 });
             } else {
-                return Err(Error::Unknown {
+                return Err(algorithm::Error::Unknown {
                     name: name.into_owned(),
                 });
             };
@@ -171,6 +196,15 @@ pub(super) mod validate {
         config::tree::{keys, Diff},
     };
 
+    pub struct Ignore;
+    impl keys::Validate for Ignore {
+        fn validate(&self, value: &BStr) -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
+            gix_submodule::config::Ignore::try_from(value)
+                .map_err(|()| format!("Value '{value}' is not a valid submodule 'ignore' value"))?;
+            Ok(())
+        }
+    }
+
     pub struct Algorithm;
     impl keys::Validate for Algorithm {
         fn validate(&self, value: &BStr) -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {

gix/src/status/index_worktree.rs

@@ -206,9 +206,11 @@ pub struct BuiltinSubmoduleStatus {
 mod submodule_status {
     use std::borrow::Cow;
 
+    use crate::config::cache::util::ApplyLeniency;
     use crate::{
         bstr,
-        bstr::{BStr, BString},
+        bstr::BStr,
+        config,
         status::{index_worktree::BuiltinSubmoduleStatus, Submodule},
     };
 
@@ -247,8 +249,8 @@ mod submodule_status {
         SubmoduleStatus(#[from] crate::submodule::status::Error),
         #[error(transparent)]
         IgnoreConfig(#[from] crate::submodule::config::Error),
-        #[error("The value of 'diff.submoduleIgnore' was invalid: '{actual}'")]
-        DiffSubmoduleIgnoreConfig { actual: BString },
+        #[error(transparent)]
+        DiffSubmoduleIgnoreConfig(#[from] config::key::GenericErrorWithValue),
     }
 
     impl gix_status::index_as_worktree::traits::SubmoduleStatus for BuiltinSubmoduleStatus {
@@ -279,14 +281,13 @@ mod submodule_status {
             let (ignore, check_dirty) = match self.mode {
                 Submodule::AsConfigured { check_dirty } => {
                     // diff.ignoreSubmodules is the global setting, and if it exists, it overrides the submodule's own ignore setting.
-                    let global_ignore = repo.config_snapshot().string("diff.ignoreSubmodules");
-                    if let Some(ignore_str) = global_ignore {
-                        let ignore = ignore_str
-                            .as_ref()
-                            .try_into()
-                            .map_err(|_| Error::DiffSubmoduleIgnoreConfig {
-                                actual: ignore_str.into_owned(),
-                            })?;
+                    let global_ignore = repo
+                        .config_snapshot()
+                        .string(&config::tree::Diff::IGNORE_SUBMODULES)
+                        .map(|value| config::tree::Diff::IGNORE_SUBMODULES.try_into_ignore(value))
+                        .transpose()
+                        .with_leniency(repo.config.lenient_config)?;
+                    if let Some(ignore) = global_ignore {
                         (ignore, check_dirty)
                     } else {
                         // If no global ignore is set, use the submodule's ignore setting.

gix/tests/gix/submodule.rs

@@ -187,7 +187,7 @@ mod open {
 
         #[test]
         fn modified_in_index_only() -> crate::Result {
-            let repo = repo("submodule-index-changed")?;
+            let mut repo: gix::Repository = repo("submodule-index-changed")?;
             let sm = repo.submodules()?.into_iter().flatten().next().expect("one submodule");
 
             for mode in [
@@ -213,6 +213,29 @@ mod open {
                 repo.is_dirty()?,
                 "superproject should see submodule changes in the index as well"
             );
+
+            repo.config_snapshot_mut()
+                .set_value(&gix::config::tree::Diff::IGNORE_SUBMODULES, "all")?;
+
+            if cfg!(feature = "parallel") {
+                assert!(!repo.is_dirty()?, "There is a global flag to deactivate this");
+            } else {
+                assert!(
+                    repo.is_dirty()?,
+                    "We still spawn a thread in non-parallel mode, \
+                    but then ThreadSafe repository isn't actually threadsafe.\
+                    This is why we open a new repo, and can't see in-memory overrides.\
+                    Maybe ThreadSafeRepository should be changed to always use thred-safe primitives."
+                );
+            }
+
+            let sm = repo.submodules()?.into_iter().flatten().next().expect("one submodule");
+            let status = sm.status_opts(gix::submodule::config::Ignore::None, true, &mut |platform| platform)?;
+            assert_eq!(
+                status.is_dirty(),
+                Some(true),
+                "The global override has no bearing on this specific call"
+            );
             Ok(())
         }