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.

Sebastian Thiel committed 3 years ago

Commit 25c5bce1 parent 4ef9a323 files · +19 −40Browse files

Unified · Split

3 files

gix-submodule/src/access.rs +2 −2
gix-submodule/src/is_active_platform.rs +7 −26
gix-submodule/tests/file/mod.rs +10 −12

gix-submodule/src/access.rs+2−2

@@ -56,13 +56,13 @@ impl File { · 55 unmodified lines
56	56	) -> bool
57	57	+ 'a,
58	58	) -> Result<
59		− impl Iterator<Item = (&BStr, Result<bool, crate::is_active_platform::is_active::Error>)> + 'a,
	59	+ impl Iterator<Item = (&BStr, Result<bool, gix_config::value::Error>)> + 'a,
60	60	crate::is_active_platform::Error,
61	61	> {
62	62	let mut platform = self.is_active_platform(config, defaults)?;
63	63	let iter = self
64	64	.names()
65		− .map(move \|name\| (name, platform.is_active(self, config, name, &mut attributes)));
	65	+ .map(move \|name\| (name, platform.is_active(config, name, &mut attributes)));
66	66	Ok(iter)
67	67	}
68	68

gix-submodule/src/is_active_platform.rs+7−26

@@ -12,34 +12,19 @@ pub enum Error { · 11 unmodified lines
12	12	ParsePattern(#[from] gix_pathspec::parse::Error),
13	13	}
14	14
15		−///
16		−pub mod is_active {
17		− /// The error returned by the iterator of [File::names_and_active_state](crate::File::names_and_active_state()).
18		− #[derive(Debug, thiserror::Error)]
19		− #[allow(missing_docs)]
20		− pub enum Error {
21		− #[error("The value of the 'active' field of a submodule could not be decoded")]
22		− ActiveField(#[from] gix_config::value::Error),
23		− #[error(transparent)]
24		− Url(#[from] crate::config::url::Error),
25		− }
26		−}
27		−
28	15	impl IsActivePlatform {
29	16	/// Returns `true` if the submodule named `name` is active or `false` otherwise.
30		− /// `modules` is the instance that [is_active_platform()](crate::File::is_active_platform()) was called on, and
31		− /// `config` is the configuration that was passed there as well.
32		− /// `attributes(relative_path, case, is_dir, outcome)` provides a way to resolve the attributes mentioned in `submodule.active` pathspecs
33		− /// that are evaluated in the platforms git configuration.
	17	+ /// `config` is the configuration that was passed to the originating [modules file](crate::File).
	18	+ /// `attributes(relative_path, case, is_dir, outcome)` provides a way to resolve the attributes mentioned
	19	+ /// in `submodule.active` pathspecs that are evaluated in the platforms git configuration.
34	20	///
35	21	/// A submodule's active state is determined in the following order
36	22	///
37		− /// * it's `submodule.<name>.active` configuration is set
	23	+ /// * it's `submodule.<name>.active` is set in `config`
38	24	/// * it matches a `submodule.active` pathspec either positively or negatively via `:!<spec>`
39		− /// * it's active if it has a `url`
	25	+ /// * it's active if it has any `url` set in `config`
40	26	pub fn is_active(
41	27	&mut self,
42		− modules: &crate::File,
43	28	config: &gix_config::File<'static>,
44	29	name: &BStr,
45	30	attributes: impl FnMut(
@@ -48,7 +33,7 @@ impl IsActivePlatform { · 2 unmodified lines
48	33	bool,
49	34	&mut gix_pathspec::attributes::search::Outcome,
50	35	) -> bool,
51		− ) -> Result<bool, is_active::Error> {
	36	+ ) -> Result<bool, gix_config::value::Error> {
52	37	if let Some(val) = config.boolean("submodule", Some(name), "active").transpose()? {
53	38	return Ok(val);
54	39	};
@@ -59,10 +44,6 @@ impl IsActivePlatform { · 4 unmodified lines
59	44	}) {
60	45	return Ok(val);
61	46	}
62		− Ok(match modules.url(name) {
63		− Ok(_) => true,
64		− Err(crate::config::url::Error::Missing { .. }) => false,
65		− Err(err) => return Err(err.into()),
66		− })
	47	+ Ok(config.string("submodule", Some(name), "url").is_some())
67	48	}
68	49	}

gix-submodule/tests/file/mod.rs+10−12

@@ -46,25 +46,23 @@ mod is_active_platform { · 45 unmodified lines
46	46	.map(\|name\| {
47	47	(
48	48	name.to_str().expect("valid"),
49		− platform
50		− .is_active(module, config, name, &mut attributes)
51		− .expect("valid"),
	49	+ platform.is_active(config, name, &mut attributes).expect("valid"),
52	50	)
53	51	})
54	52	.collect())
55	53	}
56	54
57	55	#[test]
58		− fn without_any_additional_settings_all_are_active_if_they_have_a_url() -> crate::Result {
	56	+ fn without_any_additional_settings_all_are_inactive_if_they_have_a_url() -> crate::Result {
59	57	let module = multi_modules()?;
60	58	assert_eq!(
61	59	assume_valid_active_state(&module, &Default::default(), Default::default())?,
62	60	&[
63		− ("submodule", true),
64		− ("a/b", true),
65		− (".a/..c", true),
66		− ("a/d\\", true),
67		− ("a\\e", true)
	61	+ ("submodule", false),
	62	+ ("a/b", false),
	63	+ (".a/..c", false),
	64	+ ("a/d\\", false),
	65	+ ("a\\e", false)
68	66	]
69	67	);
70	68	Ok(())
@@ -77,7 +75,7 @@ mod is_active_platform { · 6 unmodified lines
77	75	assume_valid_active_state(
78	76	&module,
79	77	&gix_config::File::from_str(
80		− "[submodule.submodule]\n active = 0\n[submodule \"a/b\"]\n active = false"
	78	+ "[submodule.submodule]\n active = 0\n url = set \n[submodule \"a/b\"]\n active = false \n url = set \n[submodule \".a/..c\"] active = 1"
81	79	)?,
82	80	Default::default()
83	81	)?,
@@ -85,8 +83,8 @@ mod is_active_platform { · 1 unmodified line
85	83	("submodule", false),
86	84	("a/b", false),
87	85	(".a/..c", true),
88		− ("a/d\\", true),
89		− ("a\\e", true)
	86	+ ("a/d\\", false),
	87	+ ("a\\e", false)
90	88	]
91	89	);
92	90	Ok(())

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

Keyboard shortcuts

General

Issues and pull requests

Issues, pull requests and Inbox

Repository code