MODULES-3624 Allow setting indent character (#237)

* MODULES-3624 Allow setting indent character
Add a new param 'indent' which allows the character used for indentation
of newly-inserted values to be set instead of just using a space. Some
software requires tabs in ini files, for example.
The default is ' ', so the default behaviour is unchanged. This change
only affects settings which are actually created. Settings which already
exist and have a new value will not be re-indented.
A limitation of this approach is that it will use whatever you specify
regardless of which characters the existing indentation uses. That's how
it behaves now with spaces, it might just be less obvious when one is
specifying explicitly.
* Start adding support for specified indent char
* Allow setting of both indent_char and indent_width
* Update existing setting test to check indent_width does nothing
* Allow indent_width to be a string containing an integer

README.markdown

@@ -90,6 +90,27 @@ default:
    minage = 1
 ~~~
 
+### Use a non-standard indent character (or string) for added settings
+
+~~~puppet
+ini_setting { 'procedure cache size':
+  ensure         => present,
+  path           => '/var/lib/ase/config/ASE-16_0/SYBASE.cfg',
+  section        => 'SQL Server Administration',
+  setting        => 'procedure cache size',
+  value          => '15000',
+  indent_char    => "\t",
+  indent_width   => 2,
+}
+~~~
+
+Results in:
+
+~~~puppet
+[SQL Server Administration]
+		procedure cache size = 15000
+~~~
+
 ### Implement child providers
 
 You might want to create child providers that inherit the `ini_setting` provider, for one or both of these purposes:
@@ -331,6 +352,14 @@ Global show_diff configuraton takes priority over this one -
 
 *Optional.*  Designates the string that will appear after the section's name.  Default value: "]".
 
+##### `indent_char`
+
+*Optional.*  Designates the character (or string) that will be used to indent newly created settings. This will not affect settings which already exist in the file, even if they are changed. Default value: " ".
+
+##### `indent_width`
+
+*Optional.*  Designates the number of `indent_char` to indent newly inserted settings by. If this is not defined, the default is to compute the indentation automatically from existing settings in the section, or no indent if the section does not yet exist. This will not affect settings which already exist in the file, even if they are changed.
+
 ##### `refreshonly`
 
 *Optional.*  A boolean to indicate whether or not the value associated with the setting should be updated if this resource is only part of a refresh event.  Default value: **false**.

lib/puppet/provider/ini_setting/ruby.rb

@@ -110,9 +110,25 @@ def section_suffix
     end
   end
 
+  def indent_char
+    if resource.class.validattr?(:indent_char)
+      resource[:indent_char] || ' '
+    else
+      ' '
+    end
+  end
+
+  def indent_width
+    if resource.class.validattr?(:indent_width)
+      resource[:indent_width] || nil
+    else
+      nil
+    end
+  end
+
   private
   def ini_file
-    @ini_file ||= Puppet::Util::IniFile.new(file_path, separator, section_prefix, section_suffix)
+    @ini_file ||= Puppet::Util::IniFile.new(file_path, separator, section_prefix, section_suffix, indent_char, indent_width)
   end
 
 end

lib/puppet/type/ini_setting.rb

@@ -111,6 +111,17 @@ def insync?(current)
     defaultto(']')
   end
 
+  newparam(:indent_char) do
+    desc 'The character to indent new settings with.' +
+      'Defaults to \' \'.'
+    defaultto(' ')
+  end
+
+  newparam(:indent_width) do
+    desc 'The number of indent_chars to use to indent a new setting.' +
+      'Defaults to undef (autodetect).'
+  end
+
   newparam(:refreshonly) do
     desc 'A flag indicating whether or not the ini_setting should be updated '+
          'only when called as part of a refresh event'
@@ -124,5 +135,4 @@ def refresh
       provider.value = self[:value]
     end
   end
-
 end

lib/puppet/util/ini_file.rb

@@ -5,12 +5,15 @@ module Puppet
 module Util
   class IniFile
 
-    def initialize(path, key_val_separator = ' = ', section_prefix = '[', section_suffix = ']')
+    def initialize(path, key_val_separator = ' = ', section_prefix = '[', section_suffix = ']',
+                   indent_char = ' ', indent_width = nil)
 
       k_v_s = key_val_separator =~ /^\s+$/ ? ' ' : key_val_separator.strip
 
       @section_prefix = section_prefix
       @section_suffix = section_suffix
+      @indent_char = indent_char
+      @indent_width = indent_width ? indent_width.to_i : nil
 
       @@SECTION_REGEX = section_regex
       @@SETTING_REGEX = /^(\s*)([^#;\s]|[^#;\s].*?[^\s#{k_v_s}])(\s*#{k_v_s}[ \t]*)(.*)\s*$/
@@ -174,7 +177,7 @@ def save
 
           # 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}")
+            fh.puts("#{@indent_char * (@indent_width || section.indentation || 0)}#{key}#{@key_val_separator}#{value}")
           end
 
           if (whitespace_buffer.length > 0)
@@ -309,7 +312,7 @@ def insert_inline_setting_line(result, section, complete_setting)
       line_num = result[:line_num]
       match = result[:match]
       s = complete_setting
-      lines.insert(line_num + 1, "#{' ' * (section.indentation || 0 )}#{s[:setting]}#{s[:separator]}#{s[:value]}")
+      lines.insert(line_num + 1, "#{@indent_char * (@indent_width || section.indentation || 0 )}#{s[:setting]}#{s[:separator]}#{s[:value]}")
     end
 
     # Utility method; given a section index (index into the @section_names

spec/unit/puppet/provider/ini_setting/ruby_spec.rb

@@ -206,6 +206,108 @@ def self.file_path
       validate_file(expected_content, tmpfile)
     end
 
+    it "should add a missing setting to the correct section with indent_char" do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(
+          :section => 'nonstandard',
+          :setting => 'indented', :value => 'weirdly',
+          :section_prefix => '-', :section_suffix => '-',
+          :indent_char => "\t"))
+      provider = described_class.new(resource)
+      expect(provider.exists?).to be false
+      provider.create
+      expected_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
+ ; yet another comment
+
+-nonstandard-
+  shoes = purple
+		indented = weirdly
+      EOS
+      validate_file(expected_content, tmpfile)
+    end
+
+    it "should add a missing setting to the correct section indented by indent_char * indent_width" do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(
+          :section => 'nonstandard',
+          :setting => 'indented', :value => 'weirdly',
+          :section_prefix => '-', :section_suffix => '-',
+          :indent_char => "\t", :indent_width => 4))
+      provider = described_class.new(resource)
+      expect(provider.exists?).to be false
+      provider.create
+      expected_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
+ ; yet another comment
+
+-nonstandard-
+  shoes = purple
+				indented = weirdly
+      EOS
+      validate_file(expected_content, tmpfile)
+    end
+
+    it "should treat a string indent_width as an integer" do
+      resource = Puppet::Type::Ini_setting.new(common_params.merge(
+          :section => 'nonstandard',
+          :setting => 'indented', :value => 'weirdly',
+          :section_prefix => '-', :section_suffix => '-',
+          :indent_char => "\t", :indent_width => '4'))
+      provider = described_class.new(resource)
+      expect(provider.exists?).to be false
+      provider.create
+      expected_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
+ ; yet another comment
+
+-nonstandard-
+  shoes = purple
+				indented = weirdly
+      EOS
+      validate_file(expected_content, tmpfile)
+    end
+
     it "should add a missing setting to the correct section with colon" do
       resource = Puppet::Type::Ini_setting.new(common_params.merge(
           :section => 'section:sub', :setting => 'yahoo', :value => 'yippee'))
@@ -1271,6 +1373,34 @@ def self.file_path
      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
+      validate_file(expected_content, tmpfile)
+    end
+
+    it "should update an existing setting at the previous indentation regardless of indent_char and indent_width settings" do
+      resource = Puppet::Type::Ini_setting.new(
+          common_params.merge(:section => 'section:sub', :setting => 'fleezy', :value => 'flam2', :indent_char => 'ignore this', :indent_width => 10))
+      provider = described_class.new(resource)
+      expect(provider.exists?).to be true
+      provider.create
+      expected_content = <<-EOS
+# This is a comment
+     [section1]
+     ; This is also a comment
+     foo=foovalue
+
+     bar = barvalue
+     master = true
+
 [section2]
   foo= foovalue2
   baz=bazvalue