Skip to content

Commit 36cdd6c

Browse files
committed
refactor
- add `diff.ignoreSubmodules` configuration key with a strong type - add test assertions to nail most relevant behaviour.
1 parent 6693ab9 commit 36cdd6c

File tree

3 files changed

+63
-15
lines changed

3 files changed

+63
-15
lines changed

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

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ impl Diff {
1515
.with_note(
1616
"The limit is actually squared, so 1000 stands for up to 1 million diffs if fuzzy rename tracking is enabled",
1717
);
18+
19+
/// The `diff.ignoreSubmodules` key.
20+
pub const IGNORE_SUBMODULES: Ignore =
21+
Ignore::new_with_validate("ignoreSubmodules", &config::Tree::DIFF, validate::Ignore)
22+
.with_note("This setting affects only the submodule status, and thus the repository status in general.");
23+
1824
/// The `diff.renames` key.
1925
pub const RENAMES: Renames = Renames::new_renames("renames", &config::Tree::DIFF);
2026

@@ -45,6 +51,7 @@ impl Section for Diff {
4551
fn keys(&self) -> &[&dyn Key] {
4652
&[
4753
&Self::ALGORITHM,
54+
&Self::IGNORE_SUBMODULES,
4855
&Self::RENAME_LIMIT,
4956
&Self::RENAMES,
5057
&Self::DRIVER_COMMAND,
@@ -59,6 +66,9 @@ impl Section for Diff {
5966
/// The `diff.algorithm` key.
6067
pub type Algorithm = keys::Any<validate::Algorithm>;
6168

69+
/// The `diff.ignoreSubmodules` key.
70+
pub type Ignore = keys::Any<validate::Ignore>;
71+
6272
/// The `diff.renames` key.
6373
pub type Renames = keys::Any<validate::Renames>;
6474

@@ -71,12 +81,27 @@ mod algorithm {
7181
use crate::{
7282
bstr::BStr,
7383
config,
74-
config::{diff::algorithm::Error, tree::sections::diff::Algorithm},
84+
config::{
85+
diff::algorithm,
86+
key,
87+
tree::sections::diff::{Algorithm, Ignore},
88+
},
7589
};
7690

91+
impl Ignore {
92+
/// See if `value` is an actual ignore
93+
pub fn try_into_ignore(
94+
&'static self,
95+
value: Cow<'_, BStr>,
96+
) -> Result<gix_submodule::config::Ignore, key::GenericErrorWithValue> {
97+
gix_submodule::config::Ignore::try_from(value.as_ref())
98+
.map_err(|()| key::GenericErrorWithValue::from_value(self, value.into_owned()))
99+
}
100+
}
101+
77102
impl Algorithm {
78103
/// Derive the diff algorithm identified by `name`, case-insensitively.
79-
pub fn try_into_algorithm(&self, name: Cow<'_, BStr>) -> Result<gix_diff::blob::Algorithm, Error> {
104+
pub fn try_into_algorithm(&self, name: Cow<'_, BStr>) -> Result<gix_diff::blob::Algorithm, algorithm::Error> {
80105
let algo = if name.eq_ignore_ascii_case(b"myers") || name.eq_ignore_ascii_case(b"default") {
81106
gix_diff::blob::Algorithm::Myers
82107
} else if name.eq_ignore_ascii_case(b"minimal") {
@@ -88,7 +113,7 @@ mod algorithm {
88113
name: name.into_owned(),
89114
});
90115
} else {
91-
return Err(Error::Unknown {
116+
return Err(algorithm::Error::Unknown {
92117
name: name.into_owned(),
93118
});
94119
};
@@ -171,6 +196,15 @@ pub(super) mod validate {
171196
config::tree::{keys, Diff},
172197
};
173198

199+
pub struct Ignore;
200+
impl keys::Validate for Ignore {
201+
fn validate(&self, value: &BStr) -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
202+
gix_submodule::config::Ignore::try_from(value)
203+
.map_err(|()| format!("Value '{value}' is not a valid submodule 'ignore' value"))?;
204+
Ok(())
205+
}
206+
}
207+
174208
pub struct Algorithm;
175209
impl keys::Validate for Algorithm {
176210
fn validate(&self, value: &BStr) -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {

gix/src/status/index_worktree.rs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -206,9 +206,11 @@ pub struct BuiltinSubmoduleStatus {
206206
mod submodule_status {
207207
use std::borrow::Cow;
208208

209+
use crate::config::cache::util::ApplyLeniency;
209210
use crate::{
210211
bstr,
211-
bstr::{BStr, BString},
212+
bstr::BStr,
213+
config,
212214
status::{index_worktree::BuiltinSubmoduleStatus, Submodule},
213215
};
214216

@@ -247,8 +249,8 @@ mod submodule_status {
247249
SubmoduleStatus(#[from] crate::submodule::status::Error),
248250
#[error(transparent)]
249251
IgnoreConfig(#[from] crate::submodule::config::Error),
250-
#[error("The value of 'diff.submoduleIgnore' was invalid: '{actual}'")]
251-
DiffSubmoduleIgnoreConfig { actual: BString },
252+
#[error(transparent)]
253+
DiffSubmoduleIgnoreConfig(#[from] config::key::GenericErrorWithValue),
252254
}
253255

254256
impl gix_status::index_as_worktree::traits::SubmoduleStatus for BuiltinSubmoduleStatus {
@@ -279,14 +281,13 @@ mod submodule_status {
279281
let (ignore, check_dirty) = match self.mode {
280282
Submodule::AsConfigured { check_dirty } => {
281283
// diff.ignoreSubmodules is the global setting, and if it exists, it overrides the submodule's own ignore setting.
282-
let global_ignore = repo.config_snapshot().string("diff.ignoreSubmodules");
283-
if let Some(ignore_str) = global_ignore {
284-
let ignore = ignore_str
285-
.as_ref()
286-
.try_into()
287-
.map_err(|_| Error::DiffSubmoduleIgnoreConfig {
288-
actual: ignore_str.into_owned(),
289-
})?;
284+
let global_ignore = repo
285+
.config_snapshot()
286+
.string(&config::tree::Diff::IGNORE_SUBMODULES)
287+
.map(|value| config::tree::Diff::IGNORE_SUBMODULES.try_into_ignore(value))
288+
.transpose()
289+
.with_leniency(self.repo.config.lenient_config)?;
290+
if let Some(ignore) = global_ignore {
290291
(ignore, check_dirty)
291292
} else {
292293
// If no global ignore is set, use the submodule's ignore setting.

gix/tests/gix/submodule.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ mod open {
187187

188188
#[test]
189189
fn modified_in_index_only() -> crate::Result {
190-
let repo = repo("submodule-index-changed")?;
190+
let mut repo: gix::Repository = repo("submodule-index-changed")?;
191191
let sm = repo.submodules()?.into_iter().flatten().next().expect("one submodule");
192192

193193
for mode in [
@@ -213,6 +213,19 @@ mod open {
213213
repo.is_dirty()?,
214214
"superproject should see submodule changes in the index as well"
215215
);
216+
217+
repo.config_snapshot_mut()
218+
.set_value(&gix::config::tree::Diff::IGNORE_SUBMODULES, "all")?;
219+
220+
assert!(!repo.is_dirty()?, "There is a global flag to deactivate this");
221+
222+
let sm = repo.submodules()?.into_iter().flatten().next().expect("one submodule");
223+
let status = sm.status_opts(gix::submodule::config::Ignore::None, true, &mut |platform| platform)?;
224+
assert_eq!(
225+
status.is_dirty(),
226+
Some(true),
227+
"The global override has no bearing on this specific call"
228+
);
216229
Ok(())
217230
}
218231

0 commit comments

Comments
 (0)