From be80806fa990f7992f2c334622cd3e4abb6c36ca Mon Sep 17 00:00:00 2001
From: Sebastian Thiel <sebastian.thiel@icloud.com>
Date: Sun, 9 Feb 2025 18:51:13 +0100
Subject: [PATCH] fix: assure that sections can be deleted properly with
 `File::remove_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.
---
 gix-config/src/file/access/mutate.rs          |  8 ++---
 gix-config/tests/config/file/access/mutate.rs | 36 ++++++++++++++++++-
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/gix-config/src/file/access/mutate.rs b/gix-config/src/file/access/mutate.rs
index c43cf0271b3..3da5f178bf4 100644
--- a/gix-config/src/file/access/mutate.rs
+++ b/gix-config/src/file/access/mutate.rs
@@ -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();
@@ -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()
diff --git a/gix-config/tests/config/file/access/mutate.rs b/gix-config/tests/config/file/access/mutate.rs
index 48826593f6c..afef5e574cf 100644
--- a/gix-config/tests/config/file/access/mutate.rs
+++ b/gix-config/tests/config/file/access/mutate.rs
@@ -25,7 +25,7 @@ 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);
 
@@ -33,17 +33,51 @@ mod remove_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;