From c2c26de9b10c3675e1044d5571e47e195a5d0167 Mon Sep 17 00:00:00 2001 From: Chris Price Date: Sat, 20 Oct 2012 00:08:06 -0700 Subject: Respect indentation / spacing for existing sections and settings This commit adds some cosmetic functionality. The main two improvements are: * We'll now pay attention to indentation within existing sections, and when we write new settings or update existing ones, we'll match the existing indentation. * When modifying existing settings, the regex now captures a greater portion of the original line and preserves it. Basically, the original whitespacing in the line should remain intact and only the value should be modified. --- lib/puppet/util/ini_file.rb | 19 +- lib/puppet/util/ini_file/section.rb | 5 +- spec/unit/puppet/provider/ini_setting/ruby_spec.rb | 203 ++++++++++++++++++++- 3 files changed, 214 insertions(+), 13 deletions(-) diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb index 52ad32c..af9bc0e 100644 --- a/lib/puppet/util/ini_file.rb +++ b/lib/puppet/util/ini_file.rb @@ -6,7 +6,7 @@ module Util class IniFile SECTION_REGEX = /^\s*\[([\w\d\.\\\/\-\:]+)\]\s*$/ - SETTING_REGEX = /^\s*([\w\d\.\\\/\-]+)\s*=\s*([\S\s]*\S)\s*$/ + SETTING_REGEX = /^(\s*)([\w\d\.\\\/\-]+)(\s*=\s*)([\S\s]*\S)\s*$/ def initialize(path, key_val_separator = ' = ') @path = path @@ -30,7 +30,7 @@ module Util def set_value(section_name, setting, value) unless (@sections_hash.has_key?(section_name)) - add_section(Section.new(section_name, nil, nil, nil)) + add_section(Section.new(section_name, nil, nil, nil, nil)) end section = @sections_hash[section_name] @@ -77,7 +77,7 @@ module Util end section.additional_settings.each_pair do |key, value| - fh.puts("#{key}#{@key_val_separator}#{value}") + fh.puts("#{' ' * (section.indentation || 0)}#{key}#{@key_val_separator}#{value}") end end end @@ -111,12 +111,15 @@ module Util def read_section(name, start_line, line_iter) settings = {} end_line_num = nil + min_indentation = nil while true line, line_num = line_iter.peek if (line_num.nil? or match = SECTION_REGEX.match(line)) - return Section.new(name, start_line, end_line_num, settings) + return Section.new(name, start_line, end_line_num, settings, min_indentation) elsif (match = SETTING_REGEX.match(line)) - settings[match[1]] = match[2] + settings[match[2]] = match[4] + indentation = match[1].length + min_indentation = [indentation, min_indentation || indentation].min end end_line_num = line_num line_iter.next @@ -126,8 +129,8 @@ module Util def update_line(section, setting, value) (section.start_line..section.end_line).each do |line_num| if (match = SETTING_REGEX.match(lines[line_num])) - if (match[1] == setting) - lines[line_num] = "#{setting}#{@key_val_separator}#{value}" + if (match[2] == setting) + lines[line_num] = "#{match[1]}#{match[2]}#{match[3]}#{value}" end end end @@ -136,7 +139,7 @@ module Util def remove_line(section, setting) (section.start_line..section.end_line).each do |line_num| if (match = SETTING_REGEX.match(lines[line_num])) - if (match[1] == setting) + if (match[2] == setting) lines.delete_at(line_num) end end diff --git a/lib/puppet/util/ini_file/section.rb b/lib/puppet/util/ini_file/section.rb index 16f19d3..c565872 100644 --- a/lib/puppet/util/ini_file/section.rb +++ b/lib/puppet/util/ini_file/section.rb @@ -2,15 +2,16 @@ module Puppet module Util class IniFile class Section - def initialize(name, start_line, end_line, settings) + 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 end - attr_reader :name, :start_line, :end_line, :additional_settings + attr_reader :name, :start_line, :end_line, :additional_settings, :indentation def get_value(setting_name) @existing_settings[setting_name] || @additional_settings[setting_name] diff --git a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb index 8b2f8e5..dc3b7cb 100644 --- a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb +++ b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb @@ -123,7 +123,7 @@ master = true [section2] foo= foovalue2 -baz = bazvalue2 +baz=bazvalue2 url = http://192.168.1.1:8080 [section:sub] subby=bar @@ -154,7 +154,7 @@ foo= foovalue2 baz=bazvalue url = http://192.168.1.1:8080 [section:sub] -subby = foo +subby=foo #another comment ; yet another comment EOS @@ -329,7 +329,7 @@ foo = http://192.168.1.1:8080 provider.value=('yippee') validate_file(<<-EOS # This is a comment -foo = yippee +foo=yippee [section2] foo = http://192.168.1.1:8080 ; yet another comment @@ -533,4 +533,201 @@ subby=bar end end + + context "when dealing with indentation in sections" do + let(:orig_content) { + <<-EOS +# This is a comment + [section1] + ; This is also a comment + foo=foovalue + + bar = barvalue + master = true + +[section2] + foo= foovalue2 + baz=bazvalue + url = http://192.168.1.1:8080 +[section:sub] + subby=bar + #another comment + fleezy = flam + ; yet another comment + EOS + } + + it "should add a missing setting at the correct indentation when the header is aligned" do + resource = Puppet::Type::Ini_setting.new(common_params.merge( + :section => 'section1', :setting => 'yahoo', :value => 'yippee')) + provider = described_class.new(resource) + provider.exists?.should be_nil + provider.create + validate_file(<<-EOS +# This is a comment + [section1] + ; This is also a comment + foo=foovalue + + bar = barvalue + master = true + + yahoo = yippee +[section2] + foo= foovalue2 + baz=bazvalue + url = http://192.168.1.1:8080 +[section:sub] + subby=bar + #another comment + fleezy = flam + ; yet another comment + EOS + ) + end + + it "should update an existing setting at the correct indentation when the header is aligned" do + resource = Puppet::Type::Ini_setting.new( + common_params.merge(:section => 'section1', :setting => 'bar', :value => 'barvalue2')) + provider = described_class.new(resource) + provider.exists?.should be_true + provider.create + validate_file(<<-EOS +# This is a comment + [section1] + ; This is also a comment + foo=foovalue + + bar = barvalue2 + master = true + +[section2] + foo= foovalue2 + baz=bazvalue + url = http://192.168.1.1:8080 +[section:sub] + subby=bar + #another comment + fleezy = flam + ; yet another comment + EOS + ) + end + + it "should add a missing setting at the correct indentation when the header is not aligned" do + resource = Puppet::Type::Ini_setting.new(common_params.merge( + :section => 'section2', :setting => 'yahoo', :value => 'yippee')) + provider = described_class.new(resource) + provider.exists?.should be_nil + provider.create + validate_file(<<-EOS +# This is a comment + [section1] + ; This is also a comment + foo=foovalue + + bar = barvalue + master = true + +[section2] + foo= foovalue2 + baz=bazvalue + url = http://192.168.1.1:8080 + yahoo = yippee +[section:sub] + subby=bar + #another comment + fleezy = flam + ; yet another comment + EOS + ) + end + + it "should update an existing setting at the correct indentation when the header is not aligned" do + resource = Puppet::Type::Ini_setting.new( + common_params.merge(:section => 'section2', :setting => 'baz', :value => 'bazvalue2')) + provider = described_class.new(resource) + provider.exists?.should be_true + provider.create + validate_file(<<-EOS +# This is a comment + [section1] + ; This is also a comment + foo=foovalue + + bar = barvalue + master = true + +[section2] + foo= foovalue2 + baz=bazvalue2 + url = http://192.168.1.1:8080 +[section:sub] + subby=bar + #another comment + fleezy = flam + ; yet another comment + EOS + ) + end + + it "should add a missing setting at the min indentation when the section is not aligned" do + resource = Puppet::Type::Ini_setting.new( + common_params.merge(:section => 'section:sub', :setting => 'yahoo', :value => 'yippee')) + provider = described_class.new(resource) + provider.exists?.should be_nil + provider.create + validate_file(<<-EOS +# This is a comment + [section1] + ; This is also a comment + foo=foovalue + + bar = barvalue + master = true + +[section2] + foo= foovalue2 + baz=bazvalue + url = http://192.168.1.1:8080 +[section:sub] + subby=bar + #another comment + fleezy = flam + ; yet another comment + yahoo = yippee + EOS + ) + end + + it "should update an existing setting at the previous indentation when the section is not aligned" do + resource = Puppet::Type::Ini_setting.new( + common_params.merge(:section => 'section:sub', :setting => 'fleezy', :value => 'flam2')) + provider = described_class.new(resource) + provider.exists?.should be_true + provider.create + validate_file(<<-EOS +# This is a comment + [section1] + ; This is also a comment + foo=foovalue + + bar = barvalue + master = true + +[section2] + foo= foovalue2 + baz=bazvalue + url = http://192.168.1.1:8080 +[section:sub] + subby=bar + #another comment + fleezy = flam2 + ; yet another comment + EOS + ) + end + + end + end -- cgit v1.2.3 From 845fa707be7132e753f291901dd7e4d4dc48c405 Mon Sep 17 00:00:00 2001 From: Chris Price Date: Sat, 20 Oct 2012 00:24:37 -0700 Subject: Better handling of whitespace lines at ends of sections This is another bit of cosmetic functionality; prior to this commit, when adding a new setting to a section, we'd write it on the very last line before the next section, even if there was a chunk of trailing whitespace lines at the end of the existing section. This was functional, but the output was not always particularly pleasant for human consumption. This commit tweaks things so that we insert new settings just before the final chunk of whitespace lines in an existing section. This keeps things a bit cleaner. --- lib/puppet/util/ini_file.rb | 47 +++++++++++++++++++++- spec/unit/puppet/provider/ini_setting/ruby_spec.rb | 3 +- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb index af9bc0e..6038b6d 100644 --- a/lib/puppet/util/ini_file.rb +++ b/lib/puppet/util/ini_file.rb @@ -64,21 +64,57 @@ module Util def save File.open(@path, 'w') do |fh| - @section_names.each do |name| + @section_names.each_index do |index| + name = @section_names[index] section = @sections_hash[name] + # We need a buffer to cache lines that are only whitespace + whitespace_buffer = [] + if section.start_line.nil? fh.puts("\n[#{section.name}]") elsif ! section.end_line.nil? (section.start_line..section.end_line).each do |line_num| - fh.puts(lines[line_num]) + line = lines[line_num] + + # We buffer any lines that are only whitespace so that + # if they are at the end of a section, we can insert + # any new settings *before* the final chunk of whitespace + # lines. + if (line =~ /^\s*$/) + whitespace_buffer << line + else + # If we get here, we've found a non-whitespace line. + # We'll flush any cached whitespace lines before we + # write it. + flush_buffer_to_file(whitespace_buffer, fh) + fh.puts(line) + end end end section.additional_settings.each_pair do |key, value| fh.puts("#{' ' * (section.indentation || 0)}#{key}#{@key_val_separator}#{value}") end + + if (whitespace_buffer.length > 0) + flush_buffer_to_file(whitespace_buffer, fh) + else + # We get here if there were no blank lines at the end of the + # section. + # + # If we are adding a new section with a new setting, + # and if there are more sections that come after this one, + # we'll write one blank line just so that there is a little + # whitespace between the sections. + if (section.end_line.nil? && + (section.additional_settings.length > 0) && + (index < @section_names.length - 1)) + fh.puts("") + end + end + end end end @@ -176,6 +212,13 @@ module Util end end + def flush_buffer_to_file(buffer, fh) + if buffer.length > 0 + buffer.each { |l| fh.puts(l) } + buffer.clear + end + end + end end end diff --git a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb index dc3b7cb..19db4c7 100644 --- a/spec/unit/puppet/provider/ini_setting/ruby_spec.rb +++ b/spec/unit/puppet/provider/ini_setting/ruby_spec.rb @@ -361,6 +361,7 @@ foo = http://192.168.1.1:8080 provider.create validate_file(<<-EOS foo = yippee + [section2] foo = http://192.168.1.1:8080 EOS @@ -571,8 +572,8 @@ subby=bar bar = barvalue master = true - yahoo = yippee + [section2] foo= foovalue2 baz=bazvalue -- cgit v1.2.3 From e517148f4a2ab309aeb8413095791b021574b417 Mon Sep 17 00:00:00 2001 From: Chris Price Date: Sat, 20 Oct 2012 22:16:36 -0700 Subject: Add example for `ensure=absent` --- tests/ini_setting.pp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/ini_setting.pp b/tests/ini_setting.pp index 6fd7686..5e2f169 100644 --- a/tests/ini_setting.pp +++ b/tests/ini_setting.pp @@ -15,3 +15,11 @@ ini_setting { "sample setting2": ensure => present, require => Ini_setting["sample setting"], } + +ini_setting { "sample setting3": + path => '/tmp/foo.ini', + section => 'bar', + setting => 'bazsetting', + ensure => absent, + require => Ini_setting["sample setting2"], +} -- cgit v1.2.3 From f0d443fed02d870965613dd174793cc1816137a2 Mon Sep 17 00:00:00 2001 From: Chris Price Date: Sat, 20 Oct 2012 23:14:39 -0700 Subject: Refactor to clarify implementation of `save` The `save` method was previously relying on some really specific implementation details of the `section` class (when the start/end_line would be nil, etc.). This made the code a bit hard to follow. This commit introduces a few utility methods in the `section` class (`is_new_section?`, `is_global?`), and refactors the `save` method to use them... this makes the logic a little easier to follow and should hopefully make it easier to maintain. --- lib/puppet/util/ini_file.rb | 11 ++++++++--- lib/puppet/util/ini_file/section.rb | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/lib/puppet/util/ini_file.rb b/lib/puppet/util/ini_file.rb index 6038b6d..0409db2 100644 --- a/lib/puppet/util/ini_file.rb +++ b/lib/puppet/util/ini_file.rb @@ -72,9 +72,12 @@ module Util # We need a buffer to cache lines that are only whitespace whitespace_buffer = [] - if section.start_line.nil? + if (section.is_new_section?) && (! section.is_global?) fh.puts("\n[#{section.name}]") - elsif ! section.end_line.nil? + end + + if ! section.is_new_section? + # write all of the pre-existing settings (section.start_line..section.end_line).each do |line_num| line = lines[line_num] @@ -94,6 +97,7 @@ module Util end end + # write new settings, if there are any section.additional_settings.each_pair do |key, value| fh.puts("#{' ' * (section.indentation || 0)}#{key}#{@key_val_separator}#{value}") end @@ -108,7 +112,8 @@ module Util # and if there are more sections that come after this one, # we'll write one blank line just so that there is a little # whitespace between the sections. - if (section.end_line.nil? && + #if (section.end_line.nil? && + if (section.is_new_section? && (section.additional_settings.length > 0) && (index < @section_names.length - 1)) fh.puts("") diff --git a/lib/puppet/util/ini_file/section.rb b/lib/puppet/util/ini_file/section.rb index c565872..d7ff159 100644 --- a/lib/puppet/util/ini_file/section.rb +++ b/lib/puppet/util/ini_file/section.rb @@ -2,6 +2,15 @@ module Puppet module Util class IniFile class Section + # Some implementation details: + # + # * `name` will be set to the empty string for the 'global' section. + # * there will always be a 'global' section, with a `start_line` of 0, + # but if the file actually begins with a real section header on + # the first line, then the 'global' section will have an + # `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) @name = name @start_line = start_line @@ -13,6 +22,16 @@ class IniFile attr_reader :name, :start_line, :end_line, :additional_settings, :indentation + def is_global?() + @name == '' + end + + def is_new_section?() + # a new section (global or named) will always have `end_line` + # set to `nil` + @end_line.nil? + end + def get_value(setting_name) @existing_settings[setting_name] || @additional_settings[setting_name] end -- cgit v1.2.3