From dd7862eab715935565d8f46036c3a8d9b8dab22b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= <romain@blogreen.org>
Date: Sun, 31 Mar 2024 13:50:16 -1000
Subject: [PATCH] Keep sections when they become empty

When the last setting of a section was removed, the whole section was
removed unless it contained white space of comments.  In #532, this was
changed to also remove sections that only contained space (blank lines),
but it caused regressions and was reverted in #535.

For consistency, we completely suppress the auto-removal of "empty"
sections: removing the last setting of a section will not remove this
section anymore, just like what happens for sections with only blank
lines and comments.
---
 lib/puppet/util/ini_file.rb                   |  8 -----
 spec/acceptance/ini_setting_spec.rb           |  5 +--
 spec/acceptance/ini_subsetting_spec.rb        |  2 +-
 .../puppet/provider/ini_setting/ruby_spec.rb  | 32 +------------------
 .../provider/ini_subsetting/ruby_spec.rb      |  4 ++-
 5 files changed, 8 insertions(+), 43 deletions(-)

diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb
index b7529812..478cc208 100644
--- a/lib/puppet/util/ini_file.rb
+++ b/lib/puppet/util/ini_file.rb
@@ -122,14 +122,6 @@ def remove_setting(section_name, setting)
       # was modified.
       section_index = @section_names.index(section_name)
       decrement_section_line_numbers(section_index + 1)
-
-      return unless section.empty?
-
-      # By convention, it's time to remove this newly emptied out section
-      lines.delete_at(section.start_line)
-      decrement_section_line_numbers(section_index + 1)
-      @section_names.delete_at(section_index)
-      @sections_hash.delete(section.name)
     end
 
     def save
diff --git a/spec/acceptance/ini_setting_spec.rb b/spec/acceptance/ini_setting_spec.rb
index 140a35e5..93c75cc4 100644
--- a/spec/acceptance/ini_setting_spec.rb
+++ b/spec/acceptance/ini_setting_spec.rb
@@ -97,7 +97,7 @@
         subject { super().content }
 
         it { is_expected.to match %r{four = five} }
-        it { is_expected.not_to match %r{\[one\]} }
+        it { is_expected.to match %r{\[one\]} }
         it { is_expected.not_to match %r{two = three} }
       end
     end
@@ -296,7 +296,8 @@
           describe '#content' do
             subject { super().content }
 
-            it { is_expected.to be_empty }
+            it { is_expected.to match %r{\[section1\]} }
+            it { is_expected.not_to match %r{valueinsection1 = newValue} }
           end
         end
       end
diff --git a/spec/acceptance/ini_subsetting_spec.rb b/spec/acceptance/ini_subsetting_spec.rb
index 297ce4c7..af16b12d 100644
--- a/spec/acceptance/ini_subsetting_spec.rb
+++ b/spec/acceptance/ini_subsetting_spec.rb
@@ -130,7 +130,7 @@
       describe '#content' do
         subject { super().content }
 
-        it { is_expected.not_to match %r{\[one\]} }
+        it { is_expected.to match %r{\[one\]} }
         it { is_expected.not_to match %r{key =} }
         it { is_expected.not_to match %r{alphabet} }
         it { is_expected.not_to match %r{betatrons} }
diff --git a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
index a250bcde..30c5e693 100644
--- a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
+++ b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb
@@ -1002,6 +1002,7 @@ def self.file_path
           #another comment
        ; yet another comment
 
+       -nonstandard-
     INIFILE
     it 'removes a setting with pre/suffix that exists' do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'nonstandard', setting: 'shoes', ensure: 'absent', section_prefix: '-', section_suffix: '-'))
@@ -1108,37 +1109,6 @@ def self.file_path
       provider.destroy
       validate_file(expected_content_five, tmpfile)
     end
-
-    expected_content_six = <<~INIFILE
-      [section1]
-      ; This is also a comment
-      foo=foovalue
-
-      bar = barvalue
-      main = true
-      [section2]
-
-      foo= foovalue2
-      baz=bazvalue
-      url = http://192.168.1.1:8080
-      [section3]
-      # com = ment
-      uncom = ment
-      [section:sub]
-      subby=bar
-          #another comment
-       ; yet another comment
-
-       -nonstandard-
-         shoes = purple
-    INIFILE
-    it 'removes the section when removing the last line in the section' do
-      resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section4', setting: 'uncom', ensure: 'absent'))
-      provider = described_class.new(resource)
-      expect(provider.exists?).to be true
-      provider.destroy
-      validate_file(expected_content_six, tmpfile)
-    end
   end
 
   context 'when dealing with indentation in sections' do
diff --git a/spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb b/spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb
index 9236fb68..49a186a2 100644
--- a/spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb
+++ b/spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb
@@ -345,7 +345,9 @@ def validate_file(expected_content, tmpfile)
         something = else
     INIFILE
 
-    expected_content_two = ''
+    expected_content_two = <<-INIFILE
+        [main]
+    INIFILE
 
     it 'removes the subsetting when the it is empty' do
       resource = Puppet::Type::Ini_subsetting.new(common_params.merge(setting: 'reports', subsetting: 'http', subsetting_separator: ','))