some refactoring and code cleanup. Better coverage

Rakefile

@@ -95,10 +95,13 @@ begin
 
   Rcov::RcovTask.new do |t|
     t.libs << "test"
+    dot_rakes = 
     t.rcov_opts = [
-      '-x/Library', '-xRakefile', '-xrakefile', '-xpublish.rf',
+      '-xRakefile', '-xrakefile', '-xpublish.rf',
+      '-xlib/rake/contrib', '-x/Library', 
       '--text-report',
-    ]
+      '--sort coverage'
+    ] + FileList['rakelib/*.rake'].pathmap("-x%p")
     t.test_files = FileList[
       'test/test*.rb', 'test/functional.rb'
     ]

lib/rake.rb

@@ -240,6 +240,33 @@ def pathmap(spec=nil, &block)
 ##############################################################################
 module Rake
 
+  # Errors -----------------------------------------------------------
+
+  # Error indicating an ill-formed task declaration.
+  class TaskArgumentError < ArgumentError
+  end
+
+  # Error indicating a recursion overflow error in task selection.
+  class RuleRecursionOverflowError < StandardError
+    def initialize(*args)
+      super
+      @targets = []
+    end
+
+    def add_target(target)
+      @targets << target
+    end
+
+    def message
+      super + ": [" + @targets.reverse.join(' => ') + "]"
+    end
+  end
+
+  # Error indicating a problem in locating the home directory on a
+  # Win32 system.
+  class Win32HomeError < RuntimeError
+  end
+
   # --------------------------------------------------------------------------
   # Rake module singleton methods.
   #
@@ -285,9 +312,6 @@ def clone
     end
   end
 
-  class TaskArgumentError < ArgumentError
-  end
-
   ####################################################################
   # TaskAguments manage the arguments passed to a task.
   #
@@ -1151,21 +1175,6 @@ def rake_check_options(options, *optdecl)
 ######################################################################
 module Rake
 
-  class RuleRecursionOverflowError < StandardError
-    def initialize(*args)
-      super
-      @targets = []
-    end
-
-    def add_target(target)
-      @targets << target
-    end
-
-    def message
-      super + ": [" + @targets.reverse.join(' => ') + "]"
-    end
-  end
-
   # #########################################################################
   # A FileList is essentially an array with a few helper methods defined to
   # make file manipulation a bit easier.
@@ -2137,16 +2146,10 @@ def display_prerequisites
       end
     end
 
-    # Return a list of the command line options supported by the
-    # program.
-    def command_line_options
-      OPTIONS.collect { |lst| lst[0..-2] }
-    end
-
-    # Read and handle the command line options.
-    def handle_options
-      # optparse version of OPTIONS
-      op_options = [
+    # A list of all the standard options used in rake, suitable for
+    # passing to OptionParser.
+    def standard_rake_options
+      [
         ['--classic-namespace', '-C', "Put Task and FileTask in the top level namespace",
           lambda { |value|
             require 'rake/classic_namespace'
@@ -2258,25 +2261,26 @@ def handle_options
             puts "rake, version #{RAKEVERSION}"
             exit
           }
-        ],
+        ]
       ]
+    end
 
+    # Read and handle the command line options.
+    def handle_options
       options.rakelib = ['rakelib']
 
-      parsed_argv = nil
-      opts = OptionParser.new do |opts|
-        opts.banner = "rake [-f rakefile] {options} targets..."
-        opts.separator ""
-        opts.separator "Options are ..."
-
-      	opts.on_tail("-h", "--help", "-H", "Display this help message.") do
-    	  	puts opts
-    		  exit
-      	end
-
-        op_options.each { |args| opts.on(*args) }
-      	parsed_argv = opts.parse(ARGV)
+      opts = OptionParser.new
+      opts.banner = "rake [-f rakefile] {options} targets..."
+      opts.separator ""
+      opts.separator "Options are ..."
+
+      opts.on_tail("-h", "--help", "-H", "Display this help message.") do
+        puts opts
+        exit
       end
+
+      standard_rake_options.each { |args| opts.on(*args) }
+      parsed_argv = opts.parse(ARGV)
 
       # If class namespaces are requested, set the global options
       # according to the values in the options structure.
@@ -2287,9 +2291,7 @@ def handle_options
         $dryrun = options.dryrun
         $silent = options.silent
       end
-      return parsed_argv
-    rescue NoMethodError => ex
-      raise OptionParser::InvalidOption, "While parsing options, error = #{ex.class}:#{ex.message}"
+      parsed_argv
     end
 
     # Similar to the regular Ruby +require+ command, but will check
@@ -2326,7 +2328,7 @@ def raw_load_rakefile # :nodoc:
       rakefile, location = find_rakefile_location
       if (! options.ignore_system) &&
           (options.load_system || rakefile.nil?) &&
-          File.directory?(system_dir)
+          directory?(system_dir)
         puts "(in #{Dir.pwd})" unless options.silent
         Dir["#{system_dir}/*.rake"].each do |name|
           add_import name
@@ -2337,7 +2339,7 @@ def raw_load_rakefile # :nodoc:
         @rakefile = rakefile
         Dir.chdir(location)
         puts "(in #{Dir.pwd})" unless options.silent
-        $rakefile = @rakefile
+        $rakefile = @rakefile if options.classic_namespace
         load File.expand_path(@rakefile) if @rakefile && @rakefile != ''
       end
       options.rakelib.each do |rlib|
@@ -2366,14 +2368,20 @@ def standard_system_dir #:nodoc:
     # The standard directory containing system wide rake files on Win
     # 32 systems.
     def win32_system_dir #:nodoc:
-      unless File.exists?(win32home = File.join(ENV['APPDATA'], 'Rake'))
-        raise Win32HomeError, "# Unable to determine home path environment variable."
+      win32home = File.join(ENV['APPDATA'], 'Rake')
+      unless directory?(win32home)
+        raise Win32HomeError, "Unable to determine home path environment variable."
       else
         win32home
       end
     end
     private :win32_system_dir
 
+    def directory?(path)
+      File.directory?(path)
+    end
+    private :directory?
+
     # Collect the list of tasks on the command line.  If no tasks are
     # given, return a list containing only the default task.
     # Environmental assignments are processed at this time as well.

lib/rake/rdoctask.rb

@@ -91,14 +91,14 @@ def define
       task name
 
       desc "Force a rebuild of the RDOC files"
-      task paste("re", name) => [paste("clobber_", name), name]
+      task "re#{name}" => ["clobber_#{name}", name]
 
       desc "Remove rdoc products" 
-      task paste("clobber_", name) do
+      task "clobber_#{name}" do
         rm_r rdoc_dir rescue nil
       end
-
-      task :clobber => [paste("clobber_", name)]
+      
+      task :clobber => ["clobber_#{name}"]
 
       directory @rdoc_dir
       task name => [rdoc_target]

lib/rake/tasklib.rb

@@ -6,11 +6,16 @@ module Rake
 
   # Base class for Task Libraries.
   class TaskLib
-
     include Cloneable
 
-    # Make a symbol by pasting two strings together. 
-    def paste(a,b)
+    # Make a symbol by pasting two strings together.
+    #
+    # NOTE: DEPRECATED! This method is kinda stupid. I don't know why
+    # I didn't just use string interpolation. But now other task
+    # libraries depend on this so I can't remove it without breaking
+    # other people's code. So for now it stays for backwards
+    # compatibility. BUT DON'T USE IT.
+    def paste(a,b)              # :nodoc:
       (a.to_s + b.to_s).intern
     end
   end

test/test_application.rb

@@ -169,6 +169,66 @@ def test_load_from_system_rakefile
     end
   end
 
+  def test_load_from_system_rakefile_on_unix
+    flexmock(@app, :windows? => false,
+      :win32_system_dir => nil,
+      :load => nil)
+    flexmock(File).should_receive(:expand_path).with("~").and_return("/HOME")
+    flexmock(File).should_receive(:expand_path).and_return { |fn| fn }
+
+    in_environment('RAKE_SYSTEM' => nil) do
+      @app.options.rakelib = []
+      @app.instance_eval do
+        handle_options
+        options.silent = true
+        options.load_system = true
+        load_rakefile
+      end
+      assert_equal "/HOME/.rake", @app.system_dir
+    end
+  end
+
+  def test_windows
+    assert ! (@app.windows? && @app.unix?)
+  end
+
+  def test_load_from_system_rakefile_on_windows
+    flexmock(@app, :windows? => true,
+      :standard_system_dir => "XX")
+    flexmock(@app).should_receive(:directory?).with("/AD/Rake").and_return(true)
+    flexmock(@app).should_receive(:load).and_return { |fn| puts "LOADING #{fn}" }
+    in_environment('RAKE_SYSTEM' => nil, 'APPDATA' => '/AD') do
+      @app.options.rakelib = []
+      @app.instance_eval do
+        handle_options
+        options.silent = true
+        options.load_system = true
+        load_rakefile
+      end
+      assert_equal "/AD/Rake", @app.system_dir
+    end
+  end
+
+  def test_load_from_system_rakefile_on_windows_with_no_appdata
+    flexmock(@app, :windows? => true,
+      :standard_system_dir => "XX"
+      )
+    flexmock(File).should_receive(:exists?).with("/AD/Rake").and_return(false)
+    out = capture_stderr do
+      assert_raise(SystemExit) do
+        in_environment('RAKE_SYSTEM' => nil, 'APPDATA' => "/AD") do
+          @app.options.rakelib = []
+          @app.instance_eval do
+            handle_options
+            options.silent = true
+            options.load_system = true
+            load_rakefile
+          end
+        end
+      end
+    end
+  end
+
   def test_loading_imports
     mock = flexmock("loader")
     mock.should_receive(:load).with("x.dummy").once
@@ -603,3 +663,52 @@ def test_keeps_embedded_spaces
   end
 
 end
+
+class TestTaskArgumentParsing < Test::Unit::TestCase
+  include InEnvironment
+
+  def test_terminal_width_using_env
+    app = Rake::Application.new
+    in_environment('RAKE_COLUMNS' => '1234') do
+      assert_equal 1234, app.terminal_width
+    end
+  end
+
+  def test_terminal_width_using_stty
+    app = Rake::Application.new
+    flexmock(app,
+      :unix? => true,
+      :dynamic_width_stty => 1235,
+      :dynamic_width_tput => 0)
+    in_environment('RAKE_COLUMNS' => nil) do
+      assert_equal 1235, app.terminal_width
+    end
+  end
+
+  def test_terminal_width_using_tput
+    app = Rake::Application.new
+    flexmock(app,
+      :unix? => true,
+      :dynamic_width_stty => 0,
+      :dynamic_width_tput => 1236)
+    in_environment('RAKE_COLUMNS' => nil) do
+      assert_equal 1236, app.terminal_width
+    end
+  end
+
+  def test_terminal_width_using_hardcoded_80
+    app = Rake::Application.new
+    flexmock(app, :unix? => false)
+    in_environment('RAKE_COLUMNS' => nil) do
+      assert_equal 80, app.terminal_width
+    end
+  end
+
+  def test_terminal_width_with_failure
+    app = Rake::Application.new
+    flexmock(app).should_receive(:unix?).and_throw(RuntimeError)
+    in_environment('RAKE_COLUMNS' => nil) do
+      assert_equal 80, app.terminal_width
+    end
+  end
+end

test/test_tasklib.rb

@@ -0,0 +1,12 @@
+#!/usr/bin/env ruby
+
+require 'test/unit'
+require 'rake/tasklib'
+
+
+class TestTaskLib < Test::Unit::TestCase
+  def test_paste
+    tl = Rake::TaskLib.new
+    assert_equal :ab, tl.paste(:a, :b)
+  end
+end