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 committed Apr 1, 2024
1 parent 32b5f3c commit c524208
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 16 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
2 changes: 0 additions & 2 deletions lib/puppet/util/ini_file/section.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ def existing_setting?(setting_name)
@existing_settings.key?(setting_name)
end

# 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
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
4 changes: 3 additions & 1 deletion 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 @@ -1124,6 +1125,7 @@ def self.file_path
[section3]
# com = ment
uncom = ment
[section4]
[section:sub]
subby=bar
#another comment
Expand All @@ -1132,7 +1134,7 @@ def self.file_path
-nonstandard-
shoes = purple
INIFILE
it 'removes the section when removing the last line in the section' do
it 'does not remove 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
Expand Down
6 changes: 4 additions & 2 deletions spec/unit/puppet/provider/ini_subsetting/ruby_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,11 @@ 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
it 'removes the subsetting and not the section when the list is empty' do
resource = Puppet::Type::Ini_subsetting.new(common_params.merge(setting: 'reports', subsetting: 'http', subsetting_separator: ','))
provider = described_class.new(resource)
provider.destroy
Expand Down

0 comments on commit c524208

Please sign in to comment.