(#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 <matt@puppetlabs.com>

2 files changed, 67 insertions, 6 deletions

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
diff --git a/spec/unit/faces/help_spec.rb b/spec/unit/faces/help_spec.rb
index 87ff67948..1399abfef 100644
--- a/spec/unit/faces/help_spec.rb
+++ b/spec/unit/faces/help_spec.rb
@@ -66,4 +66,38 @@ describe Puppet::Faces[:help, '0.0.1'] do
       it { should have_matching_element %r{ #{name} } }
     end
   end
+
+  context "when listing legacy applications" do
+    let :help do Puppet::Faces[:help, :current] end
+
+    # If we don't, these tests are ... less than useful, because they assume
+    # it.  When this breaks you should consider ditching the entire feature
+    # and tests, but if not work out how to fake one. --daniel 2011-04-11
+    it "should have at least one legacy application" do
+      help.legacy_applications.should have_at_least(1).item
+    end
+
+    # Meh.  This is nasty, but we can't control the other list; the specific
+    # bug that caused these to be listed is annoyingly subtle and has a nasty
+    # fix, so better to have a "fail if you do something daft" trigger in
+    # place here, I think. --daniel 2011-04-11
+    %w{faces_base indirection_base}.each do |name|
+      it "should not list the #{name} application" do
+        help.legacy_applications.should_not include name
+      end
+    end
+
+    Puppet::Faces[:help, :current].legacy_applications.each do |appname|
+      it "should list #{appname} in the help output" do
+        help.help.should have_matching_element %r{ #{appname} }
+      end
+
+      summary = Puppet::Faces[:help, :current].horribly_extract_summary_from(appname)
+      if summary then
+        it "should display the summary of #{appname}" do
+          help.help.should have_matching_element %r{ #{summary}\b}
+        end
+      end
+    end
+  end
 end