From 7228f580a22cc13dc66a93d81bf57a5bad80a73f Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 11:42:25 -0700 Subject: maint: finish transition of application help to return strings. Jesse made a change, in e1191f33defcaffec5900c7122a89ca75d3a9673, to transition from printing and exiting in the help method up to returning the help data to the caller. This was part of eliminating rdoc usage from the display of help to the user. The cert application was missed, and still used the legacy "print and exit" model; this cleans that up so it matches the rest of the code. Paired-With: Matt Robinson --- lib/puppet/application/cert.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/puppet/application/cert.rb b/lib/puppet/application/cert.rb index cbd6fd610..c08775380 100644 --- a/lib/puppet/application/cert.rb +++ b/lib/puppet/application/cert.rb @@ -48,7 +48,7 @@ class Puppet::Application::Cert < Puppet::Application end def help - puts <<-HELP + <<-HELP puppet-cert(8) -- Manage certificates and requests ======== @@ -166,7 +166,6 @@ COPYRIGHT Copyright (c) 2011 Puppet Labs, LLC Licensed under the Apache 2.0 License HELP - exit end def main -- cgit From d8dfb1f143ba3e602bec387508c8dad7aab00062 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Sun, 10 Apr 2011 18:59:23 -0700 Subject: (#6962) Implement 'summary' for faces. This adds the methods to the summary builder and runtime instance to support setting and getting a summary of the face. This is a short description used to summarize the purpose of the face in help output. For example, from the help face: "Displays help about puppet subcommands" Reviewed-By: Matt Robinson --- lib/puppet/interface.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb index 07e27efa8..27b3584b9 100644 --- a/lib/puppet/interface.rb +++ b/lib/puppet/interface.rb @@ -68,8 +68,13 @@ class Puppet::Interface self.default_format = format.to_sym end - attr_accessor :type, :verb, :version, :arguments - attr_reader :name + attr_accessor :summary + def summary(value = nil) + @summary = value unless value.nil? + @summary + end + + attr_reader :name, :version def initialize(name, version, &block) unless Puppet::Interface::FaceCollection.validate_version(version) -- cgit From 1b4d7a51d10b217c7f67f3876242fff6dc694faa Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 4 Apr 2011 16:46:09 -0700 Subject: (#6962) Create the basic shape of the help face. This implements the basic help face, along with the start of the support structures; we include the basic application, and the default help action that just emits a listing of faces and other discovered stuff... Reviewed-By: Matt Robinson --- lib/puppet/application/help.rb | 7 ++++++ lib/puppet/faces/help.rb | 52 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 lib/puppet/application/help.rb create mode 100644 lib/puppet/faces/help.rb (limited to 'lib') diff --git a/lib/puppet/application/help.rb b/lib/puppet/application/help.rb new file mode 100644 index 000000000..69905af27 --- /dev/null +++ b/lib/puppet/application/help.rb @@ -0,0 +1,7 @@ +require 'puppet/application/faces_base' + +class Puppet::Application::Help < Puppet::Application::FacesBase + def render(result) + puts result.join("\n") + end +end diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb new file mode 100644 index 000000000..e986d19b3 --- /dev/null +++ b/lib/puppet/faces/help.rb @@ -0,0 +1,52 @@ +require 'puppet/faces' + +Puppet::Faces.define(:help, '0.0.1') do + summary "Displays help about puppet subcommands" + + action(:help) do + option "--version VERSION" do + desc "Which version of the interface to show help for" + end + + 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 + # should rewrite this to use those. --daniel 2011-04-04 + options = args.pop + if options.nil? or args.length > 2 then + raise ArgumentError, "help only takes two (optional) arguments, a face name, and an action" + end + + if options[:version] and options[:version] !~ /^current$/i then + version = options[:version] + else + version = :current + end + + message = [] + if args.length == 0 then + message << "Use: puppet [options] " + message << "" + message << "Available commands, from Puppet Faces:" + Puppet::Faces.faces.sort.each do |name| + face = Puppet::Faces[name, :current] + message << format(" %-15s %s", face.name, 'REVISIT: face.desc') + end + else + face = Puppet::Faces[args[0].to_sym, version] + if args[1] then + action = face.get_action args[1].to_sym + else + action = nil + end + + help = [] + face.actions.each do |action| + help << "Action: #{action}" + end + end + + message + end + end +end -- cgit From 4eccd53da90593fad1b929eeea3b5d7d252c553e Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Sun, 10 Apr 2011 19:58:43 -0700 Subject: (#6962) Implement Face#summary support for the help face. We now use the summary information available in other faces as part of emitting a list our list of available subcommands. This is seldom used in faces, but enough emit the information to prove the concept. Reviewed-By: Matt Robinson --- lib/puppet/faces/help.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb index e986d19b3..229a0dc81 100644 --- a/lib/puppet/faces/help.rb +++ b/lib/puppet/faces/help.rb @@ -27,10 +27,10 @@ Puppet::Faces.define(:help, '0.0.1') do if args.length == 0 then message << "Use: puppet [options] " message << "" - message << "Available commands, from Puppet Faces:" + message << "Available subcommands, from Puppet Faces:" Puppet::Faces.faces.sort.each do |name| face = Puppet::Faces[name, :current] - message << format(" %-15s %s", face.name, 'REVISIT: face.desc') + message << format(" %-15s %s", face.name, face.summary) end else face = Puppet::Faces[args[0].to_sym, version] -- cgit From 14b1e008c1368e6c56d68f83999472351fd3327a Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 10:57:06 -0700 Subject: (#6992) Expose available_subcommands as a class method. We previously treated the list of legacy commands as an instance property of Puppet::Util::CommandLine. This made it difficult to obtain that information outside the context of the instantiated application. In the longer term this should wither and die as we eliminate the legacy applications entirely, but until then exposing that on the class is the lightest mechanism for making that useful. Paired-With: Matt Robinson --- lib/puppet/util/command_line.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb index 52b5f81ef..2891030df 100644 --- a/lib/puppet/util/command_line.rb +++ b/lib/puppet/util/command_line.rb @@ -33,14 +33,21 @@ module Puppet File.join('puppet', 'application') end - def available_subcommands - absolute_appdirs = $LOAD_PATH.collect do |x| + def self.available_subcommands + absolute_appdirs = $LOAD_PATH.collect do |x| File.join(x,'puppet','application') end.select{ |x| File.directory?(x) } absolute_appdirs.inject([]) do |commands, dir| commands + Dir[File.join(dir, '*.rb')].map{|fn| File.basename(fn, '.rb')} end.uniq end + # available_subcommands was previously an instance method, not a class + # method, and we have an unknown number of user-implemented applications + # that depend on that behaviour. Forwarding allows us to preserve a + # backward compatible API. --daniel 2011-04-11 + def available_subcommands + self.class.available_subcommands + end def usage_message usage = "Usage: puppet command " -- cgit From 36021021b4cb11aea0a5acd35d051db52d8fc99f Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 11:19:33 -0700 Subject: (#6770) Don't pollute valid face list when #face? is called. We had two conflicting uses of the list of available faces: in the #face? method we were perfectly happy to create a top level key on any request, but didn't populate the version set. Meanwhile, in #faces we treated the set of top level keys as the absolute and correct list of all *valid* faces, leading to pain and suffering when people queried for an invalid face, but then expected to enumerate only valid faces. Paired-With: Matt Robinson --- lib/puppet/interface/face_collection.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb index 84296582c..e4eb22fa3 100644 --- a/lib/puppet/interface/face_collection.rb +++ b/lib/puppet/interface/face_collection.rb @@ -53,7 +53,11 @@ module Puppet::Interface::FaceCollection def self.face?(name, version) name = underscorize(name) - return true if @faces[name].has_key?(version) + + # Note: be careful not to accidentally create the top level key, either, + # because it will result in confusion when people try to enumerate the + # list of valid faces later. --daniel 2011-04-11 + return true if @faces.has_key?(name) and @faces[name].has_key?(version) # We always load the current version file; the common case is that we have # the expected version and any compatibility versions in the same file, @@ -106,7 +110,11 @@ module Puppet::Interface::FaceCollection # but we don't need that right now. # # So, this comment is a place-holder for that. --daniel 2011-04-06 - return !! @faces[name].has_key?(version) + # + # Note: be careful not to accidentally create the top level key, either, + # because it will result in confusion when people try to enumerate the + # list of valid faces later. --daniel 2011-04-11 + return !! (@faces.has_key?(name) and @faces[name].has_key?(version)) end def self.register(face) -- cgit From 26db6456d22b95486646ae5b8b001acb2051c4da Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 11:26:25 -0700 Subject: (#6962) render prints the rval; fix help subcommand. The face application base uses render to transform the returned object to a form where #to_s produces the output intended for the end user; we were actually printing in the method instead, leading to an extraneous 'nil' at the end of the output... Paired-With: Matt Robinson --- lib/puppet/application/help.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/application/help.rb b/lib/puppet/application/help.rb index 69905af27..b3666f12d 100644 --- a/lib/puppet/application/help.rb +++ b/lib/puppet/application/help.rb @@ -2,6 +2,6 @@ require 'puppet/application/faces_base' class Puppet::Application::Help < Puppet::Application::FacesBase def render(result) - puts result.join("\n") + result.join("\n") end end -- cgit From d13a938b0abc604a7802cf41ff75a30ad6ccd192 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 10:46:08 -0700 Subject: (#6962) Initial support for legacy applications in help. We now enumerate and print the list of legacy applications in the unadorned help action; this allows us a path to migrating forward smoothly while still providing a good user experience. Paired-With: Matt Robinson --- lib/puppet/faces/help.rb | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb index 229a0dc81..c284433d1 100644 --- a/lib/puppet/faces/help.rb +++ b/lib/puppet/faces/help.rb @@ -1,6 +1,9 @@ require 'puppet/faces' +require 'puppet/util/command_line' Puppet::Faces.define(:help, '0.0.1') do + HelpSummaryFormat = ' %-18s %s' + summary "Displays help about puppet subcommands" action(:help) do @@ -30,7 +33,18 @@ Puppet::Faces.define(:help, '0.0.1') do message << "Available subcommands, from Puppet Faces:" Puppet::Faces.faces.sort.each do |name| face = Puppet::Faces[name, :current] - message << format(" %-15s %s", face.name, face.summary) + message << format(HelpSummaryFormat, face.name, face.summary) + end + + legacy = Puppet::Util::CommandLine.available_subcommands.reject do |appname| + Puppet::Faces.face? appname.to_sym, :current + end + unless legacy.empty? then # great victory when this is true! + message << "" + message << "Available applications, soon to be ported to Faces:" + legacy.sort.each do |appname| + message << format(HelpSummaryFormat, appname, 'REVISIT: how to summarize these?') + end end else face = Puppet::Faces[args[0].to_sym, version] -- cgit From 91c29a72e2b728e2d9ba495cd34b7354faa3852b Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 12:00:06 -0700 Subject: (#6962) Extract summary from legacy applications for help. We use a dubious, but effective, regexp match on the existing man(1) style help string for the application to extract the summary data. This should properly be implemented as a summary method down in the applications themselves... Paired-With: Matt Robinson --- lib/puppet/faces/help.rb | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb index c284433d1..2eb2869a4 100644 --- a/lib/puppet/faces/help.rb +++ b/lib/puppet/faces/help.rb @@ -36,14 +36,12 @@ Puppet::Faces.define(:help, '0.0.1') do message << format(HelpSummaryFormat, face.name, face.summary) end - legacy = Puppet::Util::CommandLine.available_subcommands.reject do |appname| - Puppet::Faces.face? appname.to_sym, :current - end - unless legacy.empty? then # great victory when this is true! + unless legacy_applications.empty? then # great victory when this is true! message << "" message << "Available applications, soon to be ported to Faces:" - legacy.sort.each do |appname| - message << format(HelpSummaryFormat, appname, 'REVISIT: how to summarize these?') + legacy_applications.each do |appname| + summary = horribly_extract_summary_from appname + message << format(HelpSummaryFormat, appname, summary) end end else @@ -63,4 +61,33 @@ Puppet::Faces.define(:help, '0.0.1') do message end end + + def legacy_applications + # The list of applications, less those that are duplicated as a face. + Puppet::Util::CommandLine.available_subcommands.reject do |appname| + Puppet::Faces.face? appname.to_sym, :current or + # ...this is a nasty way to exclude non-applications. :( + %w{faces_base indirection_base}.include? appname + end.sort + end + + def horribly_extract_summary_from(appname) + begin + require "puppet/application/#{appname}" + help = Puppet::Application[appname].help.split("\n") + # Now we find the line with our summary, extract it, and return it. This + # depends on the implementation coincidence of how our pages are + # formatted. If we can't match the pattern we expect we return the empty + # string to ensure we don't blow up in the summary. --daniel 2011-04-11 + while line = help.shift do + if md = /^puppet-#{appname}\([^\)]+\) -- (.*)$/.match(line) then + return md[1] + end + end + rescue Exception + # Damn, but I hate this: we just ignore errors here, no matter what + # class they are. Meh. + end + return '' + end end -- cgit From cdc5fec3640108ad01e0285b1039dae222590339 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 15:50:24 -0700 Subject: (#6962) Implement 'summary' for actions. This extends the summary function down through the actions themselves, allowing us to display a useful summary to the user. Reviewed-By: Matt Robinson --- lib/puppet/interface/action.rb | 1 + lib/puppet/interface/action_builder.rb | 4 ++++ 2 files changed, 5 insertions(+) (limited to 'lib') diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index e4a37a1f7..302e61901 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -14,6 +14,7 @@ class Puppet::Interface::Action attr_reader :name def to_s() "#{@face}##{@name}" end + attr_accessor :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 b08c3d023..34bb3fa44 100644 --- a/lib/puppet/interface/action_builder.rb +++ b/lib/puppet/interface/action_builder.rb @@ -28,4 +28,8 @@ class Puppet::Interface::ActionBuilder option = Puppet::Interface::OptionBuilder.build(@action, *declaration, &block) @action.add_option(option) end + + def summary(text) + @action.summary = text + end end -- cgit From 657082755a20da801b4c679eff296053380c61b6 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 16:19:21 -0700 Subject: (#6962) Add summary help for actions on an individual face. We now emit the summary of actions for an individual face, in the same format as the summary of available faces. This moves forward through the feature set defined for the help subcommand. Reviewed-By: Matt Robinson --- lib/puppet/faces/help.rb | 54 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 15 deletions(-) (limited to 'lib') diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb index 2eb2869a4..17ce3ec6b 100644 --- a/lib/puppet/faces/help.rb +++ b/lib/puppet/faces/help.rb @@ -7,6 +7,8 @@ Puppet::Faces.define(:help, '0.0.1') do summary "Displays help about puppet subcommands" action(:help) do + summary "Display help about faces and their actions." + option "--version VERSION" do desc "Which version of the interface to show help for" end @@ -20,15 +22,27 @@ Puppet::Faces.define(:help, '0.0.1') do raise ArgumentError, "help only takes two (optional) arguments, a face name, and an action" end - if options[:version] and options[:version] !~ /^current$/i then - version = options[:version] - else - version = :current + version = :current + if options.has_key? :version then + if options[:version].to_s !~ /^current$/i then + version = options[:version] + else + if args.length == 0 then + raise ArgumentError, "version only makes sense when a face is given" + end + end end + # Name those parameters... + facename, actionname = args + face = facename ? Puppet::Faces[facename.to_sym, version] : nil + action = (face and actionname) ? face.get_action(actionname.to_sym) : nil + + # Finally, build up the help text. Maybe ERB would have been nicer + # after all. Oh, well. --daniel 2011-04-11 message = [] if args.length == 0 then - message << "Use: puppet [options] " + message << "Use: puppet [options] [options]" message << "" message << "Available subcommands, from Puppet Faces:" Puppet::Faces.faces.sort.each do |name| @@ -44,18 +58,28 @@ Puppet::Faces.define(:help, '0.0.1') do message << format(HelpSummaryFormat, appname, summary) end end - else - face = Puppet::Faces[args[0].to_sym, version] - if args[1] then - action = face.get_action args[1].to_sym - else - action = nil - end - help = [] - face.actions.each do |action| - help << "Action: #{action}" + message << "" + message << < ' for help on a specific subcommand action. +See 'puppet help ' for help on a specific subcommand. +See 'puppet man ' for the full man page. +Puppet v#{Puppet::PUPPETVERSION} +EOT + elsif args.length == 1 then + message << "Use: puppet #{face.name} [options] [options]" + message << "" + + message << "Available actions:" + face.actions.each do |actionname| + action = face.get_action(actionname) + message << format(HelpSummaryFormat, action.name, action.summary) end + + elsif args.length == 2 + "REVISIT: gotta write this code." + else + raise ArgumentError, "help only takes two arguments, a face name and an action" end message -- cgit From 2a87f410eae84a0a7c4f39d9c1ef742c3e7ba8fc Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 16:43:56 -0700 Subject: (#6962) Override 'render' in help to just return the string. The default behaviour is to serialize the result somehow, defaulting to displayed, render should be a no-op. This means overriding the parent method. Paired-With: Matt Robinson --- lib/puppet/application/help.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/puppet/application/help.rb b/lib/puppet/application/help.rb index b3666f12d..fd8818db0 100644 --- a/lib/puppet/application/help.rb +++ b/lib/puppet/application/help.rb @@ -1,7 +1,8 @@ +# -*- coding: utf-8 -*- require 'puppet/application/faces_base' class Puppet::Application::Help < Puppet::Application::FacesBase - def render(result) - result.join("\n") - end + # Meh. Disable the default behaviour, which is to inspect the + # string and return that – not so helpful. --daniel 2011-04-11 + def render(result) result end end -- cgit From ec988e24ce3713a4c4a31918489136f88b6945e6 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 16:47:16 -0700 Subject: (#6962) Move the logic for help layout into erb templates. Rather than hard-coding the layout of help output in the code, push it out into external templates. At the moment overriding that would require changing the ERB code next to puppet/faces/help.rb file on disk. Paired-With: Matt Robinson --- lib/puppet/faces/help.rb | 60 ++++++++++------------------------------ lib/puppet/faces/help/action.erb | 3 ++ lib/puppet/faces/help/face.erb | 7 +++++ lib/puppet/faces/help/global.erb | 20 ++++++++++++++ 4 files changed, 44 insertions(+), 46 deletions(-) create mode 100644 lib/puppet/faces/help/action.erb create mode 100644 lib/puppet/faces/help/face.erb create mode 100644 lib/puppet/faces/help/global.erb (limited to 'lib') diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb index 17ce3ec6b..c5f4b0fe6 100644 --- a/lib/puppet/faces/help.rb +++ b/lib/puppet/faces/help.rb @@ -1,9 +1,9 @@ require 'puppet/faces' require 'puppet/util/command_line' +require 'pathname' +require 'erb' Puppet::Faces.define(:help, '0.0.1') do - HelpSummaryFormat = ' %-18s %s' - summary "Displays help about puppet subcommands" action(:help) do @@ -38,54 +38,22 @@ Puppet::Faces.define(:help, '0.0.1') do face = facename ? Puppet::Faces[facename.to_sym, version] : nil action = (face and actionname) ? face.get_action(actionname.to_sym) : nil - # Finally, build up the help text. Maybe ERB would have been nicer - # after all. Oh, well. --daniel 2011-04-11 - message = [] - if args.length == 0 then - message << "Use: puppet [options] [options]" - message << "" - message << "Available subcommands, from Puppet Faces:" - Puppet::Faces.faces.sort.each do |name| - face = Puppet::Faces[name, :current] - message << format(HelpSummaryFormat, face.name, face.summary) - end - - unless legacy_applications.empty? then # great victory when this is true! - message << "" - message << "Available applications, soon to be ported to Faces:" - legacy_applications.each do |appname| - summary = horribly_extract_summary_from appname - message << format(HelpSummaryFormat, appname, summary) - end - end - - message << "" - message << < ' for help on a specific subcommand action. -See 'puppet help ' for help on a specific subcommand. -See 'puppet man ' for the full man page. -Puppet v#{Puppet::PUPPETVERSION} -EOT - elsif args.length == 1 then - message << "Use: puppet #{face.name} [options] [options]" - message << "" + template = case args.length + when 0 then erb_template 'global.erb' + when 1 then erb_template 'face.erb' + when 2 then erb_template 'action.erb' + else + fail ArgumentError, "Too many arguments to help action" + end - message << "Available actions:" - face.actions.each do |actionname| - action = face.get_action(actionname) - message << format(HelpSummaryFormat, action.name, action.summary) - end - - elsif args.length == 2 - "REVISIT: gotta write this code." - else - raise ArgumentError, "help only takes two arguments, a face name and an action" - end - - message + return ERB.new(template, nil, '%').result(binding) end end + def erb_template(name) + (Pathname(__FILE__).dirname + "help" + name).read + end + def legacy_applications # The list of applications, less those that are duplicated as a face. Puppet::Util::CommandLine.available_subcommands.reject do |appname| diff --git a/lib/puppet/faces/help/action.erb b/lib/puppet/faces/help/action.erb new file mode 100644 index 000000000..eaf131464 --- /dev/null +++ b/lib/puppet/faces/help/action.erb @@ -0,0 +1,3 @@ +Use: puppet <%= face.name %> [options] <%= action.name %> [options] + +Summary: <%= action.summary %> diff --git a/lib/puppet/faces/help/face.erb b/lib/puppet/faces/help/face.erb new file mode 100644 index 000000000..efe5fd809 --- /dev/null +++ b/lib/puppet/faces/help/face.erb @@ -0,0 +1,7 @@ +Use: puppet <%= face.name %> [options] [options] + +Available actions: +% face.actions.each do |actionname| +% action = face.get_action(actionname) + <%= action.name.to_s.ljust(16) %> <%= action.summary %> +% end diff --git a/lib/puppet/faces/help/global.erb b/lib/puppet/faces/help/global.erb new file mode 100644 index 000000000..e123367a2 --- /dev/null +++ b/lib/puppet/faces/help/global.erb @@ -0,0 +1,20 @@ +puppet [options] [options] + +Available subcommands, from Puppet Faces: +% Puppet::Faces.faces.sort.each do |name| +% face = Puppet::Faces[name, :current] + <%= face.name.to_s.ljust(16) %> <%= face.summary %> +% end + +% unless legacy_applications.empty? then # great victory when this is true! +Available applications, soon to be ported to Faces: +% legacy_applications.each do |appname| +% summary = horribly_extract_summary_from appname + <%= appname.to_s.ljust(16) %> <%= summary %> +% end +% end + +See 'puppet help ' for help on a specific subcommand action. +See 'puppet help ' for help on a specific subcommand. +See 'puppet man ' for the full man page. +Puppet v<%= Puppet::PUPPETVERSION %> -- cgit From 217c1561a86a76b9570bcc4ab0db31eb25d120fa Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 17:04:42 -0700 Subject: (#6962) Report the template filename for help render errors. We now set the filename attribute of the ERB instance, which results in any error messages showing up with the right attribution: to the file they are defined in, rather than just '(erb)'. Paired-With: Matt Robinson --- lib/puppet/faces/help.rb | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb index c5f4b0fe6..26f839735 100644 --- a/lib/puppet/faces/help.rb +++ b/lib/puppet/faces/help.rb @@ -39,19 +39,24 @@ Puppet::Faces.define(:help, '0.0.1') do action = (face and actionname) ? face.get_action(actionname.to_sym) : nil template = case args.length - when 0 then erb_template 'global.erb' - when 1 then erb_template 'face.erb' - when 2 then erb_template 'action.erb' + when 0 then erb 'global.erb' + when 1 then erb 'face.erb' + when 2 then erb 'action.erb' else fail ArgumentError, "Too many arguments to help action" end - return ERB.new(template, nil, '%').result(binding) + # Run the ERB template in our current binding, including all the local + # variables we established just above. --daniel 2011-04-11 + return template.result(binding) end end - def erb_template(name) - (Pathname(__FILE__).dirname + "help" + name).read + def erb(name) + template = (Pathname(__FILE__).dirname + "help" + name) + erb = ERB.new(template.read, nil, '%') + erb.filename = template.to_s + return erb end def legacy_applications -- cgit From 648e3c0dd0fb671b01e54cc0bca202bafc5ae934 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 17:11:25 -0700 Subject: (#6962) Better argument checking for help. We used to blink and miss the fact that we failed to load an action or face in the past; now we test for that, and fail with a clear error message when the user asks for something we can't deliver. Additionally, fix a couple of tests that were silently broken because they passed the wrong arguments, but still got some output. Paired-With: Matt Robinson --- lib/puppet/faces/help.rb | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb index 26f839735..29a4a62f7 100644 --- a/lib/puppet/faces/help.rb +++ b/lib/puppet/faces/help.rb @@ -38,13 +38,19 @@ Puppet::Faces.define(:help, '0.0.1') do face = facename ? Puppet::Faces[facename.to_sym, version] : nil action = (face and actionname) ? face.get_action(actionname.to_sym) : nil - template = case args.length - when 0 then erb 'global.erb' - when 1 then erb 'face.erb' - when 2 then erb 'action.erb' - else - fail ArgumentError, "Too many arguments to help action" - end + case args.length + when 0 then + template = erb 'global.erb' + when 1 then + face or fail ArgumentError, "Unable to load face #{facename}" + template = erb 'face.erb' + when 2 then + face or fail ArgumentError, "Unable to load face #{facename}" + action or fail ArgumentError, "Unable to load action #{actionname} from #{face}" + template = erb 'action.erb' + else + fail ArgumentError, "Too many arguments to help action" + end # Run the ERB template in our current binding, including all the local # variables we established just above. --daniel 2011-04-11 -- cgit From fe6d59528d18e9aa989f48d364868a5a105017a5 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 22:04:28 -0700 Subject: (#6962) Integrate legacy subcommands into the help face. Previously we would only emit help for a face; now we fully integrate legacy subcommands in the sense that asking for face-level help will emit their entire help text. Asking for any action subsequent will raise an error: this is an annoyingly inconsistent behaviour, but there isn't a saner definition of the change. Reviewed-By: Matt Robinson --- lib/puppet/faces/help.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb index 29a4a62f7..1d8abe20e 100644 --- a/lib/puppet/faces/help.rb +++ b/lib/puppet/faces/help.rb @@ -35,8 +35,16 @@ Puppet::Faces.define(:help, '0.0.1') do # Name those parameters... facename, actionname = args - face = facename ? Puppet::Faces[facename.to_sym, version] : nil - action = (face and actionname) ? face.get_action(actionname.to_sym) : nil + + if facename then + if legacy_applications.include? facename then + actionname and raise ArgumentError, "Legacy subcommands don't take actions" + return Puppet::Application[facename].help + else + face = Puppet::Faces[facename.to_sym, version] + actionname and action = face.get_action(actionname.to_sym) + end + end case args.length when 0 then -- cgit From 826d5dff531eb624fef91a7298932e9ec5a46231 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 23:09:18 -0700 Subject: (#6962) delegate global usage to the help face. The global --help handler (also invoked when puppet is run without any other command line options at all) used to spit out a brief and generally not so helpful message. Now that we have a help face that can provide the same information in a much more user-friendly form, we should delegate the function to that when required. Reviewed-By: Matt Robinson --- lib/puppet/util/command_line.rb | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb index 2891030df..674927842 100644 --- a/lib/puppet/util/command_line.rb +++ b/lib/puppet/util/command_line.rb @@ -49,26 +49,29 @@ module Puppet self.class.available_subcommands end - def usage_message - usage = "Usage: puppet command " - available = "Available commands are: #{available_subcommands.sort.join(', ')}" - [usage, available].join("\n") - end - def require_application(application) require File.join(appdir, application) end def execute - if subcommand_name.nil? - puts usage_message - elsif available_subcommands.include?(subcommand_name) #subcommand + if subcommand_name and available_subcommands.include?(subcommand_name) then require_application subcommand_name app = Puppet::Application.find(subcommand_name).new(self) Puppet::Plugins.on_application_initialization(:appliation_object => self) app.run + elsif execute_external_subcommand then + # Logically, we shouldn't get here, but we do, so whatever. We just + # return to the caller. How strange we are. --daniel 2011-04-11 else - abort "Error: Unknown command #{subcommand_name}.\n#{usage_message}" unless execute_external_subcommand + unless subcommand_name.nil? then + puts "Error: Unknown Puppet subcommand #{subcommand_name}.\n" + end + + # Doing this at the top of the file is natural, but causes puppet.rb + # to load too early, which causes things to break. This is a nasty + # thing, found in #7065. --daniel 2011-04-11 + require 'puppet/faces/help' + puts Puppet::Faces[:help, :current].help end end -- cgit From 7899462b2ec1114907844907c77b1742193467b9 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Mon, 11 Apr 2011 23:11:35 -0700 Subject: maint: whitespace cleanup for puppet/util/command_line. This brings bracket spacing in line with our usual coding conventions. Reviewed-By: Matt Robinson --- lib/puppet/util/command_line.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb index 674927842..fa462ee2d 100644 --- a/lib/puppet/util/command_line.rb +++ b/lib/puppet/util/command_line.rb @@ -17,12 +17,12 @@ module Puppet 'master' => 'puppetmasterd' ) - def initialize( zero = $0, argv = ARGV, stdin = STDIN ) + def initialize(zero = $0, argv = ARGV, stdin = STDIN) @zero = zero @argv = argv.dup @stdin = stdin - @subcommand_name, @args = subcommand_and_args( @zero, @argv, @stdin ) + @subcommand_name, @args = subcommand_and_args(@zero, @argv, @stdin) Puppet::Plugins.on_commandline_initialization(:command_line_object => self) end @@ -79,10 +79,10 @@ module Puppet external_command = "puppet-#{subcommand_name}" require 'puppet/util' - path_to_subcommand = Puppet::Util.which( external_command ) + path_to_subcommand = Puppet::Util.which(external_command) return false unless path_to_subcommand - system( path_to_subcommand, *args ) + system(path_to_subcommand, *args) true end @@ -92,7 +92,7 @@ module Puppet private - def subcommand_and_args( zero, argv, stdin ) + def subcommand_and_args(zero, argv, stdin) zero = File.basename(zero, '.rb') if zero == 'puppet' -- cgit From 7b4d9367b391f75983868046d30928ebc8411f50 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Tue, 12 Apr 2011 11:45:05 -0700 Subject: (#6962) Move option handling into #parse_options, not #preinit. Logically, the extra work around option parsing for faces belongs in the application parse_options method, not hidden in the step before. This commit moves that to the right place and fixes the fallout from that strange early design decision. Along the way we unify error reporting for invalid options so that all the code paths result in the same externally detected failures. Reviewed-By: Matt Robinson --- lib/puppet/application.rb | 12 ++++-------- lib/puppet/application/faces_base.rb | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 13 deletions(-) (limited to 'lib') diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb index 4c5a5a967..f3a749786 100644 --- a/lib/puppet/application.rb +++ b/lib/puppet/application.rb @@ -359,14 +359,10 @@ class Application end end - # scan command line. - begin - option_parser.parse!(self.command_line.args) - rescue OptionParser::ParseError => detail - $stderr.puts detail - $stderr.puts "Try 'puppet #{command_line.subcommand_name} --help'" - exit(1) - end + # Scan command line. We just hand any exceptions to our upper levels, + # rather than printing help and exiting, so that we can meaningfully + # respond with context-sensitive help if we want to. --daniel 2011-04-12 + option_parser.parse!(self.command_line.args) end def handlearg(opt, arg) diff --git a/lib/puppet/application/faces_base.rb b/lib/puppet/application/faces_base.rb index 288b50048..f1b77f285 100644 --- a/lib/puppet/application/faces_base.rb +++ b/lib/puppet/application/faces_base.rb @@ -1,5 +1,6 @@ require 'puppet/application' require 'puppet/faces' +require 'optparse' class Puppet::Application::FacesBase < Puppet::Application should_parse_config @@ -50,11 +51,13 @@ class Puppet::Application::FacesBase < Puppet::Application $stderr.puts "Cancelling Face" exit(0) end + end + def parse_options # We need to parse enough of the command line out early, to identify what # the action is, so that we can obtain the full set of options to parse. - # TODO: These should be configurable versions, through a global + # 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::Faces[@type, :current] @@ -88,25 +91,30 @@ class Puppet::Application::FacesBase < Puppet::Application index += 1 # ...so skip the argument. end else - raise ArgumentError, "Unknown option #{item.sub(/=.*$/, '').inspect}" + raise OptionParser::InvalidOption.new(item.sub(/=.*$/, '')) end else action = @face.get_action(item.to_sym) if action.nil? then - raise ArgumentError, "#{@face} does not have an #{item.inspect} action!" + raise OptionParser::InvalidArgument.new("#{@face} does not have an #{item} action") end @action = action end end - @action or raise ArgumentError, "No action given on the command line!" + unless @action + raise OptionParser::MissingArgument.new("No action given on the command line") + end - # Finally, we can interact with the default option code to build behaviour + # Now we can interact with the default option code to build behaviour # around the full set of options we now know we support. @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 + + # ...and invoke our parent to parse all the command line options. + super end def find_global_settings_argument(item) -- cgit