Please review pull request #390: Ticket/2.7.x/12080 add custom console logging to faces opened by (kelseyhightower)

Description:

Without this patch, Face commands do not support custom formatting of
console output.

This patch fixes this issue by enhancing the current console logging
destination with support for custom formatting of messages that
originate from Puppet::Interface.

This patch enables the ability to log warnings, errors, and notices to
stderr with better formatting and colors:

Puppet::Face.err "This error will go to stderr with nice formatting"

This patch enables the ability to color specific sections of a given string:

"This is a red #{Puppet::Face.colorize(:red, 'string'}"

Related spec tests are included.

  • Opened: Mon Jan 23 16:12:25 UTC 2012
  • Based on: puppetlabs:2.7.x (0a4d48fd26bf0aadb533a53fcff9d55ae9883f5f)
  • Requested merge: kelseyhightower:ticket/2.7.x/12080_add_custom_console_logging_to_faces (226ea867f26bf22666b9ae61cd7a5b65df26b072)

Diff follows:

diff --git a/lib/puppet/face.rb b/lib/puppet/face.rb
index f73b2fc..115d9a7 100644
--- a/lib/puppet/face.rb
+++ b/lib/puppet/face.rb
@@ -10,3 +10,26 @@
 # separate out the interests people will have.  --daniel 2011-04-07
 require 'puppet/interface'
 Puppet::Face = Puppet::Interface
+
+class Puppet::Face
+
+  # We need special logging for Face commands that output to the console.
+  # Errors and warnings should go to stderr, and all other messages should
+  # go to stdout as is. This provides some flexability which allows us to
+  # colorize messages with greater detail by calling the `colorize` method
+  # on specific parts of a message:
+  #
+  #    "This message has a #{Puppet::Face.colorize(:cyan, 'cyan')} part"
+  #
+  # We also gain the ability to log warnings, errors, and notices to the
+  # console using custom formatting and colors that enhance UX.
+  #
+  #    Puppet::Face.err "This will go to stderr with nice formatting"
+  #
+  extend Puppet::Util::Logging
+  @@console = Puppet::Util::Log.desttypes[:console].new
+
+  def self.colorize(color, msg)
+    @@console.colorize(color, msg)
+  end
+end
diff --git a/lib/puppet/util/log/destinations.rb b/lib/puppet/util/log/destinations.rb
index 0ba036c..897975e 100644
--- a/lib/puppet/util/log/destinations.rb
+++ b/lib/puppet/util/log/destinations.rb
@@ -87,32 +87,78 @@ def handle(msg)
 
 Puppet::Util::Log.newdesttype :console do
 
-
-  RED     = {:console => " [0;31m", :html => "FFA0A0"}
-  GREEN   = {:console => " [0;32m", :html => "00CD00"}
-  YELLOW  = {:console => " [0;33m", :html => "FFFF60"}
-  BLUE    = {:console => " [0;34m", :html => "80A0FF"}
-  PURPLE  = {:console => " [0;35m", :html => "FFA500"}
-  CYAN    = {:console => " [0;36m", :html => "40FFFF"}
-  WHITE   = {:console => " [0;37m", :html => "FFFFFF"}
-  HRED    = {:console => " [1;31m", :html => "FFA0A0"}
-  HGREEN  = {:console => " [1;32m", :html => "00CD00"}
-  HYELLOW = {:console => " [1;33m", :html => "FFFF60"}
-  HBLUE   = {:console => " [1;34m", :html => "80A0FF"}
-  HPURPLE = {:console => " [1;35m", :html => "FFA500"}
-  HCYAN   = {:console => " [1;36m", :html => "40FFFF"}
-  HWHITE  = {:console => " [1;37m", :html => "FFFFFF"}
-  RESET   = {:console => " [0m",    :html => ""      }
+  RED        = {:console => " [0;31m", :html => "color: #FFA0A0"}
+  GREEN      = {:console => " [0;32m", :html => "color: #00CD00"}
+  YELLOW     = {:console => " [0;33m", :html => "color: #FFFF60"}
+  BLUE       = {:console => " [0;34m", :html => "color: #80A0FF"}
+  PURPLE     = {:console => " [0;35m", :html => "color: #FFA500"}
+  CYAN       = {:console => " [0;36m", :html => "color: #40FFFF"}
+  WHITE      = {:console => " [0;37m", :html => "color: #FFFFFF"}
+  HRED       = {:console => " [1;31m", :html => "color: #FFA0A0"}
+  HGREEN     = {:console => " [1;32m", :html => "color: #00CD00"}
+  HYELLOW    = {:console => " [1;33m", :html => "color: #FFFF60"}
+  HBLUE      = {:console => " [1;34m", :html => "color: #80A0FF"}
+  HPURPLE    = {:console => " [1;35m", :html => "color: #FFA500"}
+  HCYAN      = {:console => " [1;36m", :html => "color: #40FFFF"}
+  HWHITE     = {:console => " [1;37m", :html => "color: #FFFFFF"}
+
+  BG_RED     = {:console => " [0;41m", :html => "background: #FFA0A0"}
+  BG_GREEN   = {:console => " [0;42m", :html => "background: #00CD00"}
+  BG_YELLOW  = {:console => " [0;43m", :html => "background: #FFFF60"}
+  BG_BLUE    = {:console => " [0;44m", :html => "background: #80A0FF"}
+  BG_PURPLE  = {:console => " [0;45m", :html => "background: #FFA500"}
+  BG_CYAN    = {:console => " [0;46m", :html => "background: #40FFFF"}
+  BG_WHITE   = {:console => " [0;47m", :html => "background: #FFFFFF"}
+  BG_HRED    = {:console => " [1;41m", :html => "background: #FFA0A0"}
+  BG_HGREEN  = {:console => " [1;42m", :html => "background: #00CD00"}
+  BG_HYELLOW = {:console => " [1;43m", :html => "background: #FFFF60"}
+  BG_HBLUE   = {:console => " [1;44m", :html => "background: #80A0FF"}
+  BG_HPURPLE = {:console => " [1;45m", :html => "background: #FFA500"}
+  BG_HCYAN   = {:console => " [1;46m", :html => "background: #40FFFF"}
+  BG_HWHITE  = {:console => " [1;47m", :html => "background: #FFFFFF"}
+
+  RESET      = {:console => " [0m", :html => ""}
 
   Colormap = {
-    :debug => WHITE,
-    :info => GREEN,
-    :notice => CYAN,
-    :warning => YELLOW,
-    :err => HPURPLE,
-    :alert => RED,
-    :emerg => HRED,
-    :crit => HRED
+    :debug      => WHITE,
+    :info       => GREEN,
+    :notice     => CYAN,
+    :warning    => YELLOW,
+    :err        => HPURPLE,
+    :alert      => RED,
+    :emerg      => HRED,
+    :crit       => HRED,
+
+    :red        => RED,
+    :green      => GREEN,
+    :yellow     => YELLOW,
+    :blue       => BLUE,
+    :purple     => PURPLE,
+    :cyan       => CYAN,
+    :white      => WHITE,
+    :hred       => HRED,
+    :hgreen     => HGREEN,
+    :hyellow    => HYELLOW,
+    :hblue      => HBLUE,
+    :hpurple    => HPURPLE,
+    :hcyan      => HCYAN,
+    :hwhite     => HWHITE,
+    :bg_red     => BG_RED,
+    :bg_green   => BG_GREEN,
+    :bg_yellow  => BG_YELLOW,
+    :bg_blue    => BG_BLUE,
+    :bg_purple  => BG_PURPLE,
+    :bg_cyan    => BG_CYAN,
+    :bg_white   => BG_WHITE,
+    :bg_hred    => BG_HRED,
+    :bg_hgreen  => BG_HGREEN,
+    :bg_hyellow => BG_HYELLOW,
+    :bg_hblue   => BG_HBLUE,
+    :bg_hpurple => BG_HPURPLE,
+    :bg_hcyan   => BG_HCYAN,
+    :bg_hwhite  => BG_HWHITE,
+
+    :reset      => {:console => " [m", :html => ""}
   }
 
   def colorize(level, str)
@@ -125,21 +171,29 @@ def colorize(level, str)
   end
 
   def console_color(level, str)
-    Colormap[level][:console] + str + RESET[:console]
+    Colormap[level][:console] + str.gsub(RESET[:console], Colormap[level][:console]) + RESET[:console]
   end
 
   def html_color(level, str)
-    %{<span style="color: %s">%s</span>} % [Colormap[level][:html], str]
+    %{<span style="%s">%s</span>} % [Colormap[level][:html], str]
   end
 
   def initialize
     # Flush output immediately.
+    $stderr.sync = true
     $stdout.sync = true
   end
 
   def handle(msg)
-    if msg.source == "Puppet"
+    case msg.source
+    when "Puppet"
       puts colorize(msg.level, "#{msg.level}: #{msg}")
+    when "Puppet::Interface"
+      if [:err, :warning].include?(msg.level)
+        $stderr.puts colorize(:hred, "#{msg}")
+      else
+        $stdout.puts "#{msg}"
+      end
     else
       puts colorize(msg.level, "#{msg.level}: #{msg.source}: #{msg}")
     end
diff --git a/spec/unit/util/log/destinations_spec.rb b/spec/unit/util/log/destinations_spec.rb
index 3cb8e24..b8ebed3 100755
--- a/spec/unit/util/log/destinations_spec.rb
+++ b/spec/unit/util/log/destinations_spec.rb
@@ -107,5 +107,82 @@
       klass.suitable?(:syslog).should be_false
     end
   end
+
+  describe Puppet::Util::Log.desttypes[:console] do
+    before :each do
+      Puppet[:color] = true
+    end
+
+    let (:dest) { Puppet::Util::Log.desttypes[:console].new }
+
+    describe "when color is enabled" do
+      let (:red_string)   { dest.colorize(:red, 'string') }
+      let (:reset_string) { dest.colorize(:reset, 'string') }
+
+      it "should color output" do
+        dest.colorize(:red, 'string').should == " [0;31mstring [0m"
+      end
+
+      it "should handle multiple overlapping colors in a stack-like way" do
+        dest.colorize(:green, "(#{red_string})").should == " [0;32m( [0;31mstring [0;32m) [0m"
+      end
+
+      it "should handle resets in a stack-like way" do
+        dest.colorize(:green, "(#{reset_string})").should == " [0;32m( [mstring [0;32m) [0m"
+      end
+
+      describe "when the message source is Puppet::Interface" do
+        before :each do
+          normal_msg.source  = 'Puppet::Interface'
+          warning_msg.source = 'Puppet::Interface'
+          error_msg.source   = 'Puppet::Interface'
+        end
+
+        let(:normal_msg)  { Puppet::Util::Log.new(:level => :info, :message => "Normal") }
+        let(:warning_msg) { Puppet::Util::Log.new(:level => :warning, :message => "Warning") }
+        let(:error_msg)   { Puppet::Util::Log.new(:level => :err, :message => "Error") }
+
+        it "should output normal messages to stdout" do
+          $stdout.expects(:puts)
+          dest.handle(normal_msg)
+        end
+
+        it "should output warning messages to stderr" do
+          $stderr.expects(:puts)
+          dest.handle(warning_msg)
+        end
+
+        it "should output error messages to stderr" do
+          $stderr.expects(:puts)
+          dest.handle(error_msg)
+        end
+
+        it "should not color normal messages" do
+          $stdout.expects(:puts).with("Normal")
+          dest.handle(normal_msg)
+        end
+
+        it "should color warning messages bright red" do
+          $stderr.expects(:puts).with(" [1;31mWarning [0m")
+          dest.handle(warning_msg)
+        end
+
+        it "should color error messages bright red" do
+          $stderr.expects(:puts).with(" [1;31mError [0m")
+          dest.handle(error_msg)
+        end
+      end
+    end
+
+    describe "when color is disabled" do
+      before :each do
+        Puppet[:color] = false
+      end
+
+      it "should not color output" do
+        dest.colorize(:red, 'output').should == "output"
+      end
+    end
+  end
 end
 
diff --git a/spec/unit/util/log_spec.rb b/spec/unit/util/log_spec.rb
index 68728fe..e550af1 100755
--- a/spec/unit/util/log_spec.rb
+++ b/spec/unit/util/log_spec.rb
@@ -34,7 +34,7 @@
     it "should htmlize if Puppet[:color] is :html" do
       Puppet[:color] = :html
 
-      @console.colorize(:alert, "abc").should == "<span style=\"color: FFA0A0\">abc</span>"
+      @console.colorize(:alert, "abc").should == "<span style=\"color: #FFA0A0\">abc</span>"
     end
 
     it "should do nothing if Puppet[:color] is false" do

    

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to