Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep sections when they become empty #536

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 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 Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions lib/puppet/util/ini_file/section.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
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
64 changes: 1 addition & 63 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,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
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
Loading