Skip to content

Commit

Permalink
Keep sections when they become empty
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
smortex authored and Ramesh7 committed Jun 6, 2024
1 parent 649c92f commit dd7862e
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 43 deletions.
8 changes: 0 additions & 8 deletions lib/puppet/util/ini_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions spec/acceptance/ini_setting_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/acceptance/ini_subsetting_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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} }
Expand Down
32 changes: 1 addition & 31 deletions spec/unit/puppet/provider/ini_setting/ruby_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: '-'))
Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: ','))
Expand Down

0 comments on commit dd7862e

Please sign in to comment.