(#7013) Strip out old face-wide rendering defaults.

Now we want to support action-based rendering, it is super-hard to define the
semantics around defaulting where things are unspecified: the execution
context (CLI, HTTP, etc) vs the face, vs the action all have different
semantics.

Without solving the problem of how we express all that context and those
semantics down in the action, especially one written by a third party, this
just becomes a box of counter-intuitive and annoying semantics and edge-cases.

Reviewed-By: Max Martin <max@puppetlabs.com>

9 files changed, 14 insertions, 41 deletions

diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb
index 947204d68..456f0c610 100644
--- a/lib/puppet/application/face_base.rb
+++ b/lib/puppet/application/face_base.rb
@@ -15,8 +15,8 @@ class Puppet::Application::FaceBase < Puppet::Application
     Puppet::Util::Log.level = :info
   end
 
-  option("--format FORMAT") do |arg|
-    @format = arg.to_sym
+  option("--render-as FORMAT") do |arg|
+    @render_as = arg.to_sym
   end
 
   option("--mode RUNMODE", "-r") do |arg|
@@ -26,7 +26,7 @@ class Puppet::Application::FaceBase < Puppet::Application
   end
 
 
-  attr_accessor :face, :action, :type, :arguments, :format
+  attr_accessor :face, :action, :type, :arguments, :render_as
   attr_writer :exit_code
 
   # This allows you to set the exit code if you don't want to just exit
@@ -37,8 +37,8 @@ class Puppet::Application::FaceBase < Puppet::Application
 
   # Override this if you need custom rendering.
   def render(result)
-    if format then
-      render_method = Puppet::Network::FormatHandler.format(format).render_method
+    if render_as then
+      render_method = Puppet::Network::FormatHandler.format(render_as).render_method
       if render_method == "to_pson"
         jj result
       else
@@ -85,9 +85,8 @@ class Puppet::Application::FaceBase < Puppet::Application
 
     # REVISIT: These should be configurable versions, through a global
     # '--version' option, but we don't implement that yet... --daniel 2011-03-29
-    @type   = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
-    @face   = Puppet::Face[@type, :current]
-    @format = @face.default_format
+    @type      = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
+    @face      = Puppet::Face[@type, :current]
 
     # Now, walk the command line and identify the action.  We skip over
     # arguments based on introspecting the action and all, and find the first
diff --git a/lib/puppet/face/facts.rb b/lib/puppet/face/facts.rb
index 8668b2531..04eab93a5 100644
--- a/lib/puppet/face/facts.rb
+++ b/lib/puppet/face/facts.rb
@@ -2,10 +2,10 @@ require 'puppet/face/indirector'
 require 'puppet/node/facts'
 
 Puppet::Face::Indirector.define(:facts, '0.0.1') do
-  set_default_format :yaml
-
   # Upload our facts to the server
   action(:upload) do
+    render_as :yaml
+
     when_invoked do |options|
       Puppet::Node::Facts.indirection.terminus_class = :facter
       facts = Puppet::Node::Facts.indirection.find(Puppet[:certname])
diff --git a/lib/puppet/face/node.rb b/lib/puppet/face/node.rb
index fd1a548d6..12336df8f 100644
--- a/lib/puppet/face/node.rb
+++ b/lib/puppet/face/node.rb
@@ -1,5 +1,3 @@
 require 'puppet/face/indirector'
-
 Puppet::Face::Indirector.define(:node, '0.0.1') do
-  set_default_format :yaml
 end
diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 51ae0cdc1..398d85403 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -62,10 +62,8 @@ class Puppet::Interface
     end
   end
 
-  attr_accessor :default_format
-
   def set_default_format(format)
-    self.default_format = format.to_sym
+    Puppet.warning("set_default_format is deprecated (and ineffective); use render_as on your actions instead.")
   end
 
   attr_accessor :summary
@@ -83,7 +81,6 @@ class Puppet::Interface
 
     @name = Puppet::Interface::FaceCollection.underscorize(name)
     @version = version
-    @default_format = :pson
 
     instance_eval(&block) if block_given?
   end
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 53c40bc51..8bacc216d 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -9,9 +9,9 @@ class Puppet::Interface::Action
     @name    = name.to_sym
     attrs.each do |k, v| send("#{k}=", v) end
 
-    @options = {}
+    @options        = {}
     @when_rendering = {}
-    @render_as = :for_humans
+    @render_as      = :for_humans
   end
 
   # This is not nice, but it is the easiest way to make us behave like the
diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb
index 0d39aa1ff..6a4647246 100755
--- a/spec/unit/application/face_base_spec.rb
+++ b/spec/unit/application/face_base_spec.rb
@@ -59,10 +59,6 @@ describe Puppet::Application::FaceBase do
           app.face.name.should == :basetest
         end
 
-        it "should set the format based on the face default" do
-          app.format.should == :pson
-        end
-
         it "should find the action" do
           app.action.should be
           app.action.name.should == :foo
@@ -192,7 +188,6 @@ describe Puppet::Application::FaceBase do
 
       app.face      = Puppet::Face[:basetest, '0.0.1']
       app.action    = app.face.get_action(:foo)
-      app.format    = :pson
       app.arguments = ["myname", "myarg"]
     end
 
diff --git a/spec/unit/face/facts_spec.rb b/spec/unit/face/facts_spec.rb
index e6411f836..6ab6ad5be 100755
--- a/spec/unit/face/facts_spec.rb
+++ b/spec/unit/face/facts_spec.rb
@@ -2,14 +2,10 @@
 require 'spec_helper'
 
 describe Puppet::Face[:facts, '0.0.1'] do
-  it "should define an 'upload' fact" do
+  it "should define an 'upload' action" do
     subject.should be_action(:upload)
   end
 
-  it "should set its default format to :yaml" do
-    subject.default_format.should == :yaml
-  end
-
   describe "when uploading" do
     it "should set the terminus_class to :facter"
 
diff --git a/spec/unit/face/node_spec.rb b/spec/unit/face/node_spec.rb
index 90d258db9..d19312c58 100755
--- a/spec/unit/face/node_spec.rb
+++ b/spec/unit/face/node_spec.rb
@@ -2,7 +2,5 @@
 require 'spec_helper'
 
 describe Puppet::Face[:node, '0.0.1'] do
-  it "should set its default format to :yaml" do
-    subject.default_format.should == :yaml
-  end
+  it "REVISIT: really should have some tests"
 end
diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb
index e52b45d8a..799b8c4b4 100755
--- a/spec/unit/interface_spec.rb
+++ b/spec/unit/interface_spec.rb
@@ -99,16 +99,6 @@ describe Puppet::Interface do
     subject.new(:me, '0.0.1').to_s.should =~ /\bme\b/
   end
 
-  it "should allow overriding of the default format" do
-    face = subject.new(:me, '0.0.1')
-    face.set_default_format :foo
-    face.default_format.should == :foo
-  end
-
-  it "should default to :pson for its format" do
-    subject.new(:me, '0.0.1').default_format.should == :pson
-  end
-
   # Why?
   it "should create a class-level autoloader" do
     subject.autoloader.should be_instance_of(Puppet::Util::Autoload)