fix!: IsActivePlatform::is_active() now only considers the URL in passed configuration

Previously, it would consider the presence of `url` in the `.gitmodules` file as well, which is
not the way git does it.

Author: Byron

gix-submodule/src/access.rs

@@ -56,13 +56,13 @@ impl File {
             ) -> bool
             + 'a,
     ) -> Result<
-        impl Iterator<Item = (&BStr, Result<bool, crate::is_active_platform::is_active::Error>)> + 'a,
+        impl Iterator<Item = (&BStr, Result<bool, gix_config::value::Error>)> + 'a,
         crate::is_active_platform::Error,
     > {
         let mut platform = self.is_active_platform(config, defaults)?;
         let iter = self
             .names()
-            .map(move |name| (name, platform.is_active(self, config, name, &mut attributes)));
+            .map(move |name| (name, platform.is_active(config, name, &mut attributes)));
         Ok(iter)
     }
 

gix-submodule/src/is_active_platform.rs

@@ -12,34 +12,19 @@ pub enum Error {
     ParsePattern(#[from] gix_pathspec::parse::Error),
 }
 
-///
-pub mod is_active {
-    /// The error returned by the iterator of [File::names_and_active_state](crate::File::names_and_active_state()).
-    #[derive(Debug, thiserror::Error)]
-    #[allow(missing_docs)]
-    pub enum Error {
-        #[error("The value of the 'active' field of a submodule could not be decoded")]
-        ActiveField(#[from] gix_config::value::Error),
-        #[error(transparent)]
-        Url(#[from] crate::config::url::Error),
-    }
-}
-
 impl IsActivePlatform {
     /// Returns `true` if the submodule named `name` is active or `false` otherwise.
-    /// `modules` is the instance that [is_active_platform()](crate::File::is_active_platform()) was called on, and
-    /// `config` is the configuration that was passed there as well.
-    /// `attributes(relative_path, case, is_dir, outcome)` provides a way to resolve the attributes mentioned in `submodule.active` pathspecs
-    /// that are evaluated in the platforms git configuration.
+    /// `config` is the configuration that was passed to the originating [modules file](crate::File).
+    /// `attributes(relative_path, case, is_dir, outcome)` provides a way to resolve the attributes mentioned
+    /// in `submodule.active` pathspecs that are evaluated in the platforms git configuration.
     ///
     /// A submodule's active state is determined in the following order
     ///
-    /// * it's `submodule.<name>.active` configuration is set
+    /// * it's `submodule.<name>.active` is set in `config`
     /// * it matches a `submodule.active` pathspec either positively or negatively via `:!<spec>`
-    /// * it's active if it has a `url`
+    /// * it's active if it has any `url` set in `config`
     pub fn is_active(
         &mut self,
-        modules: &crate::File,
         config: &gix_config::File<'static>,
         name: &BStr,
         attributes: impl FnMut(
@@ -48,7 +33,7 @@ impl IsActivePlatform {
             bool,
             &mut gix_pathspec::attributes::search::Outcome,
         ) -> bool,
-    ) -> Result<bool, is_active::Error> {
+    ) -> Result<bool, gix_config::value::Error> {
         if let Some(val) = config.boolean("submodule", Some(name), "active").transpose()? {
             return Ok(val);
         };
@@ -59,10 +44,6 @@ impl IsActivePlatform {
         }) {
             return Ok(val);
         }
-        Ok(match modules.url(name) {
-            Ok(_) => true,
-            Err(crate::config::url::Error::Missing { .. }) => false,
-            Err(err) => return Err(err.into()),
-        })
+        Ok(config.string("submodule", Some(name), "url").is_some())
     }
 }

gix-submodule/tests/file/mod.rs

@@ -46,25 +46,23 @@ mod is_active_platform {
             .map(|name| {
                 (
                     name.to_str().expect("valid"),
-                    platform
-                        .is_active(module, config, name, &mut attributes)
-                        .expect("valid"),
+                    platform.is_active(config, name, &mut attributes).expect("valid"),
                 )
             })
             .collect())
     }
 
     #[test]
-    fn without_any_additional_settings_all_are_active_if_they_have_a_url() -> crate::Result {
+    fn without_any_additional_settings_all_are_inactive_if_they_have_a_url() -> crate::Result {
         let module = multi_modules()?;
         assert_eq!(
             assume_valid_active_state(&module, &Default::default(), Default::default())?,
             &[
-                ("submodule", true),
-                ("a/b", true),
-                (".a/..c", true),
-                ("a/d\\", true),
-                ("a\\e", true)
+                ("submodule", false),
+                ("a/b", false),
+                (".a/..c", false),
+                ("a/d\\", false),
+                ("a\\e", false)
             ]
         );
         Ok(())
@@ -77,16 +75,16 @@ mod is_active_platform {
             assume_valid_active_state(
                 &module,
                 &gix_config::File::from_str(
-                    "[submodule.submodule]\n active = 0\n[submodule \"a/b\"]\n active = false"
+                    "[submodule.submodule]\n active = 0\n url = set \n[submodule \"a/b\"]\n active = false \n url = set \n[submodule \".a/..c\"] active = 1"
                 )?,
                 Default::default()
             )?,
             &[
                 ("submodule", false),
                 ("a/b", false),
                 (".a/..c", true),
-                ("a/d\\", true),
-                ("a\\e", true)
+                ("a/d\\", false),
+                ("a\\e", false)
             ]
         );
         Ok(())