Skip to content

Commit

Permalink
fix: assure that sections can be deleted properly with `File::remove_…
Browse files Browse the repository at this point in the history
…section_filter()` (#1826)

The underlying issue isn't fixed, as ids in the lookup table remain even after deletion.
However, for now it was easiest to defensively access this data, while assuring that
the API (still) does what it claims.
  • Loading branch information
Byron committed Feb 10, 2025
1 parent 9bec947 commit be80806
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 6 deletions.
8 changes: 3 additions & 5 deletions gix-config/src/file/access/mutate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,8 @@ impl<'event> File<'event> {
.section_ids_by_name_and_subname(name.as_ref(), subsection_name)
.ok()
.and_then(|it| {
it.rev().find(|id| {
let s = &self.sections[id];
filter(s.meta())
})
it.rev()
.find(|id| self.sections.get(id).is_some_and(|s| filter(s.meta())))
}) {
Some(id) => {
let nl = self.detect_newline_style_smallvec();
Expand Down Expand Up @@ -305,7 +303,7 @@ impl<'event> File<'event> {
.section_ids_by_name_and_subname(name, subsection_name)
.ok()?
.rev()
.find(|id| filter(self.sections.get(id).expect("each id has a section").meta()))?;
.find(|id| self.sections.get(id).is_some_and(|section| filter(section.meta())))?;
self.section_order.remove(
self.section_order
.iter()
Expand Down
36 changes: 35 additions & 1 deletion gix-config/tests/config/file/access/mutate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,59 @@ mod remove_section {
}

#[test]
fn removal_is_complete_and_sections_can_be_readded() {
fn removal_is_complete_and_sections_can_be_read() {
let mut file = gix_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap();
assert_eq!(file.sections().count(), 2);

let removed = file.remove_section("core", None).expect("removed correct section");
assert_eq!(removed.header().name(), "core");
assert_eq!(removed.header().subsection_name(), None);
assert_eq!(file.sections().count(), 1);
assert_eq!(file.remove_section("core", None), None, "it's OK to try again");

let removed = file.remove_section("core", Some("name".into())).expect("found");
assert_eq!(removed.header().name(), "core");
assert_eq!(removed.header().subsection_name().expect("present"), "name");
assert_eq!(file.sections().count(), 0);
assert_eq!(file.remove_section("core", Some("name".into())), None);

file.section_mut_or_create_new("core", None).expect("creation succeeds");
file.section_mut_or_create_new("core", Some("name".into()))
.expect("creation succeeds");
}
}
mod remove_section_filter {
#[test]
fn removal_of_section_is_complete() {
let mut file = gix_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap();
assert_eq!(file.sections().count(), 2);

let removed = file
.remove_section_filter("core", None, |_| true)
.expect("removed correct section");
assert_eq!(removed.header().name(), "core");
assert_eq!(removed.header().subsection_name(), None);
assert_eq!(file.sections().count(), 1);
let removed = file
.remove_section_filter("core", Some("name".into()), |_| true)
.expect("found");
assert_eq!(removed.header().name(), "core");
assert_eq!(removed.header().subsection_name().expect("present"), "name");
assert_eq!(file.sections().count(), 0);

assert_eq!(
file.remove_section_filter("core", None, |_| true),
None,
"it's OK to try again"
);
assert_eq!(file.remove_section_filter("core", Some("name".into()), |_| true), None);

file.section_mut_or_create_new("core", None).expect("creation succeeds");
file.section_mut_or_create_new("core", Some("name".into()))
.expect("creation succeeds");
}
}

mod rename_section {
use std::borrow::Cow;

Expand Down

0 comments on commit be80806

Please sign in to comment.