rework more stuff to make reek happy

lib/apache/apachify.rb

@@ -26,17 +26,34 @@ class String
   include Apache::Apachify
 
   alias :optionify :apachify
+
+  def commentize
+    self.split("\n")
+  end
+
+  def quoteize
+    %{"#{self}"}
+  end
 end
 
 # Ruby symbols
 class Symbol
   include Apache::Apachify
 
+  # Turn this into an option for IndexOptions
   def optionify
     output = self.apachify
     output = "-#{output[4..-1].apachify}" if self.to_s[0..3] == 'not_'
     output
   end
+
+  def quoteize
+    to_s.gsub('_', ' ')
+  end
+end
+
+class Fixnum
+  def quoteize; to_s; end
 end
 
 # Ruby arrays
@@ -45,4 +62,14 @@ class Array
   def apachify
     self.collect(&:apachify)
   end
+
+  def quoteize
+    self.collect(&:quoteize)
+  end
+
+  def quoteize!
+    self.collect!(&:quoteize)
+  end
+
+  alias :commentize :to_a
 end

lib/apache/config.rb

@@ -57,7 +57,6 @@ module Apache
       attr_accessor :line_indent, :rotate_logs_path
 
       include Apache::Master
-      include Apache::Quoteize
       include Apache::Permissions
       include Apache::Directories
       include Apache::Logging
@@ -132,7 +131,7 @@ module Apache
         if method_name[-1..-1] == "!"
           method = method_name[0..-2].to_sym
         else
-          args = *quoteize(*args)
+          args.quoteize!
         end
 
         self << [ method.apachify, *args ].compact * ' '
@@ -195,9 +194,9 @@ module Apache
       def blockify_name(name)
         case name
           when String
-            quoteize(name).first
+            name.quoteize.first
           when Array
-            (quoteize(*name) * " ")
+            name.quoteize * " "
           when Symbol
             name.to_s
         end

lib/apache/directory.rb

@@ -6,7 +6,7 @@ module Apache
     # The options passed into this method are Apachified:
     #  options :exec_cgi, :follow_sym_links #=> Options ExecCGI FollowSymLinks
     def options(*opt)
-      create_options_list('Options', *opt)
+      create_options_list('options'.apachify, *opt)
     end
 
     # Create an IndexOptions directive
@@ -14,7 +14,7 @@ module Apache
     # The options passed into this method are Apachified:
     #  index_options :fancy_indexing, :suppress_description #=> IndexOptions FancyIndexing SuppressDescription
     def index_options(*opt)
-      create_options_list('IndexOptions', *opt)
+      create_options_list('index_options'.apachify, *opt)
     end
 
     private

lib/apache/logging.rb

@@ -19,11 +19,11 @@ module Apache
     [ :custom, :error, :script, :rewrite ].each do |type|
       class_eval <<-EOT
         def #{type}_log(*opts)
-          handle_log '#{type.to_s.capitalize}Log', opts.first, quoteize(opts.first), opts[1..-1]
+          handle_log '#{type.to_s.capitalize}Log', opts.first, opts.first.quoteize, opts[1..-1]
         end
 
         def rotate_#{type}_log(*opts)
-          handle_log '#{type.to_s.capitalize}Log', opts.first, quoteize(rotatelogs(*opts[0..1])), opts[2..-1]
+          handle_log '#{type.to_s.capitalize}Log', opts.first, rotatelogs(*opts[0..1]).quoteize, opts[2..-1]
         end
       EOT
     end

lib/apache/master.rb

@@ -15,7 +15,7 @@ module Apache
     #    Listen "1.2.3.4:80"
     #    Listen "2.3.4.5:80"
     def listen(*opt)
-      opt.each { |o| self << "Listen #{quoteize(o)}" }
+      opt.each { |adapter| self << "Listen #{adapter.quoteize}" }
     end
     alias :listen! :listen
 
@@ -48,27 +48,19 @@ module Apache
     end
 
     # Set the TCP timeout. Defined here to get around various other timeout methods.
-    def timeout(t)
+    def timeout(time)
-      self << "Timeout #{t}"
+      self << "Timeout #{time}"
     end
 
     # Add a comment to the Apache config. Can pass in either a String or Array of comment lines.
-    def comment(c)
+    def comment(what)
-      out = [ '' ]
+      self + [ '', what.commentize, '' ].flatten.collect { |line| "# #{line.strip}".strip }
-      case c
-        when String
-          out += c.split("\n")
-        when Array
-          out += c
-      end
-      out << ''
-      self + out.collect { |line| "# #{line.strip}".strip }
     end
 
     # Create a ScriptAlias, checking to make sure the filesystem path exists.
     def script_alias(uri, path)
       directory? path
-      self << %{ScriptAlias #{quoteize(uri, path) * ' '}}
+      self << %{ScriptAlias #{[ uri, path ].quoteize * ' '}}
     end
 
     alias :script_alias! :script_alias
@@ -92,7 +84,7 @@ module Apache
     # Alias a URL to a directory in the filesystem.
     # Used to get around reserved Ruby keyword.
     def apache_alias(*opts)
-      self << "Alias #{quoteize(*opts) * " "}"
+      self << "Alias #{opts.quoteize * " "}"
     end
 
     # Set multiple headers to be delivered for a particular section
@@ -102,12 +94,12 @@ module Apache
     #  Header set "Content-dispoaition" "attachment" env=only-for-downloads
     def set_header(hash)
       hash.each do |key, value|
-        output = "Header set #{quoteize(key)}"
+        output = "Header set #{key.quoteize}"
         case value
           when String, Symbol
-            output += " #{quoteize(value)}"
+            output += " #{value.quoteize}"
           when Array
-            output += " #{quoteize(value.first)} #{value.last}"
+            output += " #{value.first.quoteize} #{value.last}"
         end
         self << output
       end
@@ -115,14 +107,15 @@ module Apache
 
     def server_name(*opts)
       if first = opts.shift
-        self << "ServerName #{quoteize(first)}"
+        self << "ServerName #{first.quoteize}"
-        opts.each { |o| server_alias o } if !opts.empty?
+        opts.each { |name| server_alias name } if !opts.empty?
       end
     end
 
     def document_root(*opts)
-      directory? opts.first
+      dir = opts.first
-      self << "DocumentRoot #{quoteize(opts.first)}"
+      directory? dir
+      self << "DocumentRoot #{dir.quoteize}"
     end
     alias :document_root! :document_root
   end

lib/apache/modules.rb

@@ -1,11 +1,7 @@
-require 'apache/quoteize'
-
 module Apache
   # Create lists of modules to load in the Apache 2.2 style (with LoadModule only)
   class Modules
     class << self
-      include Apache::Quoteize
-
       attr_accessor :modules
 
       # Reset the list of modules to output
@@ -27,7 +23,7 @@ module Apache
       def build(*modules, &block)
         reset!
 
-        modules.each { |m| self.send(m) }
+        modules.each { |mod| self.send(mod) }
         self.instance_eval(&block) if block
 
         [ '' ] + @modules + [ '' ]
@@ -37,7 +33,7 @@ module Apache
       def method_missing(method, *args)
         module_name = "#{method}_module"
         module_path = args[0] || "modules/mod_#{method}.so"
-        @modules << [ 'LoadModule', *quoteize(module_name, module_path) ] * " "
+        @modules << [ 'LoadModule', *[ module_name, module_path ].quoteize ] * " "
       end
     end
   end

lib/apache/permissions.rb

@@ -21,7 +21,7 @@ module Apache
     #
     #  allow_from '127.0.0.1' #=> Allow from "127.0.0.1"
     def allow_from(*where)
-      self << "Allow from #{quoteize(*where) * " "}"
+      self << "Allow from #{where.quoteize * " "}"
     end
 
     # Specify default access order
@@ -58,26 +58,18 @@ module Apache
     #  basic_authentication "My other secret", '/my.users', :user => [ :john ]
     def basic_authentication(zone, users_file, requires = {})
       exist? users_file
-      auth_type :basic
+      authentication_basics(zone, requires)
-      auth_name zone
       auth_user_file users_file
-      requires.each do |type, values|
-        apache_require type, *values
-      end
     end
 
     alias :basic_authentication! :basic_authentication
 
     # Set up LDAP authentication
     def ldap_authentication(zone, url, requires = {})
-      auth_type :basic
+      authentication_basics(zone, requires)
-      auth_name zone
       auth_basic_provider :ldap
       authz_ldap_authoritative :on
       auth_ldap_url url
-      requires.each do |type, values|
-        apache_require type, *values
-      end
     end
 
     alias :ldap_authentication! :ldap_authentication
@@ -87,5 +79,14 @@ module Apache
     def apache_require(*opts)
       self << "Require #{opts.compact * " "}"
     end
+
+    private
+      def authentication_basics(zone, requires)
+        auth_type :basic
+        auth_name zone
+        requires.each do |type, values|
+          apache_require type, *values
+        end
+      end
   end
 end

lib/apache/quoteize.rb

@@ -1,16 +0,0 @@
-module Apache
-  # Add quotes around parameters as needed
-  module Quoteize
-    # Add quotes around most parameters, and don't add quotes around Symbols
-    def quoteize(*args)
-      args.collect do |arg|
-        case arg
-          when Symbol
-            arg.to_s.gsub('_', ' ')
-          else
-            %{"#{arg}"}
-        end
-      end
-    end
-  end
-end

lib/apache/rewrites.rb

@@ -27,7 +27,7 @@ module Apache
     end
 
     def rewrite(*opt, &block)
-      raise "You probably want rewrites #{quoteize(*opt) * " "} do" if block
+      raise "You probably want rewrites #{opt.quoteize * " "} do" if block
     end
 
     # Create a permanent Redirect
@@ -37,7 +37,7 @@ module Apache
       if opt.first && !opt.first.kind_of?(::String)
         raise "First parameter should be a String. Did you mean to wrap this in a rewrites block? #{opt.first}"
       end
-      self << "Redirect permanent #{quoteize(*opt) * " "}"
+      self << "Redirect permanent #{opt.quoteize * " "}"
     end
   end
 
@@ -192,8 +192,6 @@ module Apache
 
   # A matchable thing to be extended
   class MatchableThing
-    include Apache::Quoteize
-
     attr_reader :from, :to
 
     # The Apache directive tag for this thing
@@ -210,7 +208,7 @@ module Apache
     end
 
     def to_s
-      "#{tag} #{[quoteize(@from), quoteize(@to)].compact.flatten * " "}"
+      "#{tag} #{[@from, @to].quoteize.compact.flatten * " "}"
     end
 
     def to_a
@@ -275,7 +273,7 @@ module Apache
     end
 
     def to_s
-      "#{tag} #{[quoteize(@from.source), quoteize(@to), @options].compact.flatten * " "}"
+      "#{tag} #{[@from.source.quoteize, @to.quoteize, @options].compact.flatten * " "}"
     end
 
     def to_a
@@ -329,7 +327,7 @@ module Apache
     end
 
     def to_s
-      "#{tag} #{[quoteize(@from.source), quoteize(@to)].compact.flatten * " "}"
+      "#{tag} #{[@from.source, @to].quoteize.compact.flatten * " "}"
     end
 
     def stop_if_match; true; end
@@ -370,7 +368,7 @@ module Apache
     end
 
     def to_s
-      "#{tag} #{[quoteize(@from), quoteize(@to), @options].compact.flatten * " "}"
+      "#{tag} #{[@from.quoteize, @to.quoteize, @options].compact.flatten * " "}"
     end
 
     # Test this RewriteCond

spec/config_spec.rb

@@ -41,8 +41,8 @@ describe Apache::Config, "builds configurations" do
   end
 
   it "should quoteize properly" do
-    apache.quoteize("test", "test2").should == %w{"test" "test2"}
+    ["test", "test2"].quoteize.should == %w{"test" "test2"}
-    apache.quoteize(:test, :test2).should == %w{test test2}
+    [:test, :test2].quoteize.should == %w{test test2}
   end
 
   it "should blockify a block" do