Merge branch 'tickets/2.7.x/7115' into 2.7.x

8 files changed, 98 insertions, 16 deletions

diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb
index 2a048a532..df828b5a2 100644
--- a/lib/puppet/application/face_base.rb
+++ b/lib/puppet/application/face_base.rb
@@ -39,7 +39,6 @@ class Puppet::Application::FaceBase < Puppet::Application
     render_method = Puppet::Network::FormatHandler.format(format).render_method
     if render_method == "to_pson"
       jj result
-      exit(0)
     else
       result.send(render_method)
     end
@@ -94,16 +93,13 @@ class Puppet::Application::FaceBase < Puppet::Application
           raise OptionParser::InvalidOption.new(item.sub(/=.*$/, ''))
         end
       else
-        action = @face.get_action(item.to_sym)
-        if action.nil? then
-          raise OptionParser::InvalidArgument.new("#{@face} does not have an #{item} action")
-        end
-        @action = action
+        @action = @face.get_action(item.to_sym)
       end
     end
 
-    unless @action
-      raise OptionParser::MissingArgument.new("No action given on the command line")
+    if @action.nil?
+      @action = @face.get_default_action()
+      @is_default_action = true
     end
 
     # Now we can interact with the default option code to build behaviour
@@ -111,7 +107,7 @@ class Puppet::Application::FaceBase < Puppet::Application
     @action.options.each do |option|
       option = @action.get_option(option) # make it the object.
       self.class.option(*option.optparse) # ...and make the CLI parse it.
-    end
+    end if @action
 
     # ...and invoke our parent to parse all the command line options.
     super
@@ -138,7 +134,10 @@ class Puppet::Application::FaceBase < Puppet::Application
     # with it *always* being the first word of the remaining set of command
     # line arguments.  So, strip that off when we construct the arguments to
     # pass down to the face action. --daniel 2011-04-04
-    @arguments.delete_at(0)
+    # Of course, now that we have default actions, we should leave the
+    # "action" name on if we didn't actually consume it when we found our
+    # action.
+    @arguments.delete_at(0) unless @is_default_action
 
     # We copy all of the app options to the end of the call; This allows each
     # action to read in the options.  This replaces the older model where we
@@ -150,8 +149,17 @@ class Puppet::Application::FaceBase < Puppet::Application
 
   def main
     # Call the method associated with the provided action (e.g., 'find').
-    if result = @face.send(@action.name, *arguments)
-      puts render(result)
+    if @action
+      result = @face.send(@action.name, *arguments)
+      puts render(result) if result
+    else
+      if arguments.first.is_a? Hash
+        puts "#{@face} does not have a default action"
+      else
+        puts "#{@face} does not respond to action #{arguments.first}"
+      end
+
+      puts Puppet::Face[:help, :current].help(@face.name, *arguments)
     end
     exit(exit_code)
   end
diff --git a/lib/puppet/face/help.rb b/lib/puppet/face/help.rb
index 1c2da9e83..8bd495f8a 100644
--- a/lib/puppet/face/help.rb
+++ b/lib/puppet/face/help.rb
@@ -13,6 +13,7 @@ Puppet::Face.define(:help, '0.0.1') do
       desc "Which version of the interface to show help for"
     end
 
+    default
     when_invoked do |*args|
       # Check our invocation, because we want varargs and can't do defaults
       # yet.  REVISIT: when we do option defaults, and positional options, we
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index db338e39e..3c18c2aaf 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -23,7 +23,7 @@ class Puppet::Interface::Action
   attr_reader :name
   def to_s() "#{@face}##{@name}" end
 
-  attr_accessor :summary
+  attr_accessor :default, :summary
 
   # Initially, this was defined to allow the @action.invoke pattern, which is
   # a very natural way to invoke behaviour given our introspection
diff --git a/lib/puppet/interface/action_builder.rb b/lib/puppet/interface/action_builder.rb
index 34bb3fa44..639d8fc7f 100644
--- a/lib/puppet/interface/action_builder.rb
+++ b/lib/puppet/interface/action_builder.rb
@@ -29,6 +29,10 @@ class Puppet::Interface::ActionBuilder
     @action.add_option(option)
   end
 
+  def default
+    @action.default = true
+  end
+
   def summary(text)
     @action.summary = text
   end
diff --git a/lib/puppet/interface/action_manager.rb b/lib/puppet/interface/action_manager.rb
index d75697afa..440be2d1b 100644
--- a/lib/puppet/interface/action_manager.rb
+++ b/lib/puppet/interface/action_manager.rb
@@ -5,8 +5,13 @@ module Puppet::Interface::ActionManager
   # the code to do so.
   def action(name, &block)
     @actions ||= {}
+    @default_action ||= nil
     raise "Action #{name} already defined for #{self}" if action?(name)
     action = Puppet::Interface::ActionBuilder.build(self, name, &block)
+    if action.default
+      raise "Actions #{@default_action.name} and #{name} cannot both be default" if @default_action
+      @default_action = action
+    end
     @actions[action.name] = action
   end
 
@@ -50,6 +55,10 @@ module Puppet::Interface::ActionManager
     return result
   end
 
+  def get_default_action
+    @default_action
+  end
+
   def action?(name)
     actions.include?(name.to_sym)
   end
diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb
index 939712ef8..eaf60b434 100755
--- a/spec/unit/application/face_base_spec.rb
+++ b/spec/unit/application/face_base_spec.rb
@@ -70,9 +70,33 @@ describe Puppet::Application::FaceBase do
         end
       end
 
-      it "should fail if no action is given" do
-        expect { app.preinit; app.parse_options }.
-          to raise_error OptionParser::MissingArgument, /No action given/
+      it "should use the default action if not given any arguments" do
+        app.command_line.stubs(:args).returns []
+        action = stub(:options => [])
+        Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(action)
+        app.stubs(:main)
+        app.run
+        app.action.should == action
+        app.arguments.should == [ { } ]
+      end
+
+      it "should use the default action if not given a valid one" do
+        app.command_line.stubs(:args).returns %w{bar}
+        action = stub(:options => [])
+        Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(action)
+        app.stubs(:main)
+        app.run
+        app.action.should == action
+        app.arguments.should == [ 'bar', { } ]
+      end
+
+      it "should have no action if not given a valid one and there is no default action" do
+        app.command_line.stubs(:args).returns %w{bar}
+        Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(nil)
+        app.stubs(:main)
+        app.run
+        app.action.should be_nil
+        app.arguments.should == [ 'bar', { } ]
       end
 
       it "should report a sensible error when options with = fail" do
@@ -178,6 +202,13 @@ describe Puppet::Application::FaceBase do
       app.main
     end
 
+    it "should lookup help when it cannot do anything else" do
+      app.action = nil
+      Puppet::Face[:help, :current].expects(:help).with(:basetest, *app.arguments)
+      app.stubs(:puts)          # meh.  Don't print nil, thanks. --daniel 2011-04-12
+      app.main
+    end
+
     it "should use its render method to render any result" do
       app.expects(:render).with(app.arguments.length + 1)
       app.stubs(:puts)          # meh.  Don't print nil, thanks. --daniel 2011-04-12
diff --git a/spec/unit/interface/action_builder_spec.rb b/spec/unit/interface/action_builder_spec.rb
index 5b04df900..8f29c8a7b 100755
--- a/spec/unit/interface/action_builder_spec.rb
+++ b/spec/unit/interface/action_builder_spec.rb
@@ -65,5 +65,16 @@ describe Puppet::Interface::ActionBuilder do
         action.summary.should == "this is some text"
       end
     end
+
+    context "action defaulting" do
+      let :face do Puppet::Interface.new(:default_action, '0.0.1') end
+
+      it "should set the default to true" do
+        action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+          default
+        end
+        action.default.should == true
+      end
+    end
   end
 end
diff --git a/spec/unit/interface/action_manager_spec.rb b/spec/unit/interface/action_manager_spec.rb
index c4b21eaac..387faa043 100755
--- a/spec/unit/interface/action_manager_spec.rb
+++ b/spec/unit/interface/action_manager_spec.rb
@@ -3,6 +3,7 @@ require 'spec_helper'
 
 # This is entirely an internal class for Interface, so we have to load it instead of our class.
 require 'puppet/interface'
+require 'puppet/face'
 
 class ActionManagerTester
   include Puppet::Interface::ActionManager
@@ -213,6 +214,23 @@ describe Puppet::Interface::ActionManager do
     end
   end
 
+  describe "#action" do
+    it 'should add an action' do
+      subject.action(:foo) {  }
+      subject.get_action(:foo).should be_a Puppet::Interface::Action
+    end
+
+    it 'should support default actions' do
+      subject.action(:foo) { default }
+      subject.get_default_action.should == subject.get_action(:foo)
+    end
+
+    it 'should not support more than one default action' do
+      subject.action(:foo) { default }
+      expect { subject.action(:bar) { default } }.should raise_error
+    end
+  end
+
   describe "#get_action" do
     let :parent_class do
       parent_class = Class.new(Puppet::Interface)