Merge pull request #536 from puppetlabs/524-ini_setting-keep-sections

Keep sections when they become empty

Author: span786

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
@@ -222,24 +214,21 @@ def read_section(name, start_line, line_iter)
       end_line_num = start_line
       min_indentation = nil
       empty = true
-      empty_line_count = 0
       loop do
         line, line_num = line_iter.peek
         if line_num.nil? || @section_regex.match(line)
           # the global section always exists, even when it's empty;
           # when it's empty, we must be sure it's thought of as new,
           # which is signalled with a nil ending line
           end_line_num = nil if name == '' && empty
-          return Section.new(name, start_line, end_line_num, settings, min_indentation, empty_line_count)
+          return Section.new(name, start_line, end_line_num, settings, min_indentation)
         end
         if (match = @setting_regex.match(line))
           settings[match[2]] = match[4]
           indentation = match[1].length
           min_indentation = [indentation, min_indentation || indentation].min
         end
         end_line_num = line_num
-        empty_line_count += 1 if line == "\n"
-
         empty = false
         line_iter.next
       end

lib/puppet/util/ini_file/section.rb

@@ -14,14 +14,13 @@ class Section
     #    `end_line` of `nil`.
     #  * `start_line` and `end_line` will be set to `nil` for a new non-global
     #    section.
-    def initialize(name, start_line, end_line, settings, indentation, empty_line_count = 0)
+    def initialize(name, start_line, end_line, settings, indentation)
       @name = name
       @start_line = start_line
       @end_line = end_line
       @existing_settings = settings.nil? ? {} : settings
       @additional_settings = {}
       @indentation = indentation
-      @empty_line_count = empty_line_count
     end
 
     attr_reader :name, :start_line, :end_line, :additional_settings, :indentation
@@ -51,7 +50,7 @@ def existing_setting?(setting_name)
     # the global section is empty whenever it's new;
     # other sections are empty when they have no lines
     def empty?
-      global? ? new_section? : (start_line == end_line || (end_line && (end_line - @empty_line_count)) == start_line)
+      global? ? new_section? : start_line == end_line
     end
 
     def update_existing_setting(setting_name, value)

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

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} }

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,69 +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 section has only empty line' do
-    let(:orig_content) do
-      <<~INIFILE
-        [section1]
-        foo=foovalue
-
-
-        [section2]
-
-        foo= foovalue2
-        baz=bazvalue
-        url = http://
-      INIFILE
-    end
-
-    expected_content = <<~INIFILE
-      [section2]
-
-      foo= foovalue2
-      baz=bazvalue
-      url = http://
-    INIFILE
-
-    it 'remove empty section' do
-      resource = Puppet::Type::Ini_setting.new(common_params.merge(section: 'section1', setting: 'foo', ensure: 'absent'))
-      provider = described_class.new(resource)
-      expect(provider.exists?).to be true
-      provider.destroy
-      validate_file(expected_content, tmpfile)
-    end
   end
 
   context 'when dealing with indentation in sections' do

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: ','))