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.

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