Please review pull request #485: (#12339) Module Search Formatting -- now with more 1.8.5 compatibility! opened by (pvande)
Description:
Tested interactively in RHEL 5 (Ruby 1.8.5).
- Opened: Fri Feb 10 01:11:24 UTC 2012
- Based on: puppetlabs:master (5bb758bb3035f40c436887c89ac64138a7162ad7)
- Requested merge: pvande:tickets/2.7.x/12339-module-search-formatting (c9dadbb268e80fc6f6f1ecf0b7b2c37fb34664c8)
Diff follows:
diff --git a/lib/puppet/face/module/search.rb b/lib/puppet/face/module/search.rb
index cec8d90..771a080 100644
--- a/lib/puppet/face/module/search.rb
+++ b/lib/puppet/face/module/search.rb
@@ -1,3 +1,5 @@
+require 'puppet/util/terminal'
+
Puppet::Face.define(:module, '1.0.0') do
action(:search) do
summary "Search a repository for a module."
@@ -29,27 +31,59 @@
Puppet::Module::Tool::Applications::Searcher.run(term, options)
end
- when_rendering :console do |return_value|
+ when_rendering :console do |results, term, options|
+ return '' if results.empty?
- FORMAT = "%-10s %-32s %-14s %s\n"
+ padding = ' '
+ headers = {
+ 'full_name' => 'NAME',
+ 'desc' => 'DESCRIPTION',
+ 'author' => 'AUTHOR',
+ 'tag_list' => 'KEYWORDS',
+ }
- def header
- FORMAT % ['NAME', 'DESCRIPTION', 'AUTHOR', 'KEYWORDS']
- end
+ min_widths = Hash[ *headers.map { |k,v| [k, v.length] }.flatten ]
+ min_widths['full_name'] = min_widths['author'] = 12
+
+ min_width = min_widths.inject(0) { |sum,pair| sum += pair.last } + (padding.length * (headers.length - 1))
- def format_row(name, description, author, tag_list)
- keywords = tag_list.join(' ')
- FORMAT % [name[0..10], description[0..32], "@#{author[0..14]}", keywords]
+ terminal_width = [Puppet::Util::Terminal.width, min_width].max
+
+ columns = results.inject(min_widths) do |hash, result|
+ {
+ 'full_name' => [ hash['full_name'], result['full_name'].length ].max,
+ 'desc' => [ hash['desc'], result['desc'].length ].max,
+ 'author' => [ hash['author'], "@#{result['author']}".length ].max,
+ 'tag_list' => [ hash['tag_list'], result['tag_list'].join(' ').length ].max,
+ }
end
- output = ''
- output << header unless return_value.empty?
+ flex_width = terminal_width - columns['full_name'] - columns['author'] - (padding.length * (headers.length - 1))
+ tag_lists = results.map { |r| r['tag_list'] }
- return_value.map do |match|
- output << format_row(match['name'], match['desc'], match['author'], match['tag_list'])
+ while (columns['tag_list'] > flex_width / 3)
+ longest_tag_list = tag_lists.sort_by { |tl| tl.join(' ').length }.last
+ break if [ [], [term] ].include? longest_tag_list
+ longest_tag_list.delete(longest_tag_list.sort_by { |t| t == term ? -1 : t.length }.last)
+ columns['tag_list'] = tag_lists.map { |tl| tl.join(' ').length }.max
end
- output
+ columns['tag_list'] = [
+ flex_width / 3,
+ tag_lists.map { |tl| tl.join(' ').length }.max,
+ ].max
+ columns['desc'] = flex_width - columns['tag_list']
+
+ format = %w{full_name desc author tag_list}.map do |k|
+ "%-#{ [ columns[k], min_widths[k] ].max }s"
+ end.join(padding) + "\n"
+
+ format % [ headers['full_name'], headers['desc'], headers['author'], headers['tag_list'] ] +
+ results.map do |match|
+ name, desc, author, keywords = %w{full_name desc author tag_list}.map { |k| match[k] }
+ desc = desc[0...(columns['desc'] - 3)] + '...' if desc.length > columns['desc']
+ format % [name.sub('/', '-'), desc, "@#{author}", [keywords].flatten.join(' ')]
+ end.join
end
end
end
diff --git a/lib/puppet/util/terminal.rb b/lib/puppet/util/terminal.rb
new file mode 100644
index 0000000..c0d32e3
--- /dev/null
+++ b/lib/puppet/util/terminal.rb
@@ -0,0 +1,17 @@
+module Puppet::Util::Terminal
+ # Attempts to determine the width of the terminal. This is currently only
+ # supported on POSIX systems, and relies on the claims of `stty` (or `tput`).
+ #
+ # Inspired by code from Thor; thanks wycats!
+ # @return [Number] The column width of the terminal. Defaults to 80 columns.
+ def self.width
+ if Puppet.features.posix?
+ result = %x{stty size 2>/dev/null}.split[1] ||
+ %x{tput cols 2>/dev/null}.split[0] ||
+ '80'
+ end
+ return result.to_i
+ rescue
+ return 80
+ end
+end
diff --git a/spec/unit/face/module/search_spec.rb b/spec/unit/face/module/search_spec.rb
index 250c5e0..668ad49 100644
--- a/spec/unit/face/module/search_spec.rb
+++ b/spec/unit/face/module/search_spec.rb
@@ -1,5 +1,7 @@
require 'spec_helper'
require 'puppet/face'
+require 'puppet/application/module'
+require 'puppet/module_tool'
describe "puppet module search" do
subject { Puppet::Face[:module, :current] }
@@ -8,6 +10,135 @@
{}
end
+ describe Puppet::Application::Module do
+ subject do
+ app = Puppet::Application::Module.new
+ app.stubs(:action).returns(Puppet::Face.find_action(:module, :search))
+ app
+ end
+
+ before { subject.render_as = :console }
+ before { Puppet::Util::Terminal.stubs(:width).returns(100) }
+
+ it 'should output nothing when receiving an empty dataset' do
+ subject.render([], ['apache', {}]).should == ''
+ end
+
+ it 'should output a header when receiving a non-empty dataset' do
+ results = [
+ {'full_name' => '', 'author' => '', 'desc' => '', 'tag_list' => [] },
+ ]
+
+ subject.render(results, ['apache', {}]).should =~ /NAME/
+ subject.render(results, ['apache', {}]).should =~ /DESCRIPTION/
+ subject.render(results, ['apache', {}]).should =~ /AUTHOR/
+ subject.render(results, ['apache', {}]).should =~ /KEYWORDS/
+ end
+
+ it 'should output the relevant fields when receiving a non-empty dataset' do
+ results = [
+ {'full_name' => 'Name', 'author' => 'Author', 'desc' => 'Summary', 'tag_list' => ['tag1', 'tag2'] },
+ ]
+
+ subject.render(results, ['apache', {}]).should =~ /Name/
+ subject.render(results, ['apache', {}]).should =~ /Author/
+ subject.render(results, ['apache', {}]).should =~ /Summary/
+ subject.render(results, ['apache', {}]).should =~ /tag1/
+ subject.render(results, ['apache', {}]).should =~ /tag2/
+ end
+
+ it 'should elide really long descriptions' do
+ results = [
+ {
+ 'full_name' => 'Name',
+ 'author' => 'Author',
+ 'desc' => 'This description is really too long to fit in a single data table, guys -- we should probably set about truncating it',
+ 'tag_list' => ['tag1', 'tag2'],
+ },
+ ]
+
+ subject.render(results, ['apache', {}]).should =~ /\.{3} @Author/
+ end
+
+ it 'should never truncate the module name' do
+ results = [
+ {
+ 'full_name' => 'This-module-has-a-really-really-long-name',
+ 'author' => 'Author',
+ 'desc' => 'Description',
+ 'tag_list' => ['tag1', 'tag2'],
+ },
+ ]
+
+ subject.render(results, ['apache', {}]).should =~ /This-module-has-a-really-really-long-name/
+ end
+
+ it 'should never truncate the author name' do
+ results = [
+ {
+ 'full_name' => 'Name',
+ 'author' => 'This-author-has-a-really-really-long-name',
+ 'desc' => 'Description',
+ 'tag_list' => ['tag1', 'tag2'],
+ },
+ ]
+
+ subject.render(results, ['apache', {}]).should =~ /@This-author-has-a-really-really-long-name/
+ end
+
+ it 'should never remove tags that match the search term' do
+ results = [
+ {
+ 'full_name' => 'Name',
+ 'author' => 'Author',
+ 'desc' => 'Description',
+ 'tag_list' => ['Supercalifragilisticexpialidocious'] + (1..100).map { |i| "tag#{i}" },
+ },
+ ]
+
+ subject.render(results, ['Supercalifragilisticexpialidocious', {}]).should =~ /Supercalifragilisticexpialidocious/
+ subject.render(results, ['Supercalifragilisticexpialidocious', {}]).should_not =~ /tag/
+ end
+
+ {
+ 100 => <<-EOT,
+NAME DESCRIPTION AUTHOR KEYWORDS
+Name This description is really too long to fit ... @JohnnyApples tag1 tag2 taggitty3
+EOT
+
+ 70 => <<-EOT,
+NAME DESCRIPTION AUTHOR KEYWORDS
+Name This description is rea... @JohnnyApples tag1 tag2
+EOT
+
+ 80 => <<-EOT,
+NAME DESCRIPTION AUTHOR KEYWORDS
+Name This description is really too... @JohnnyApples tag1 tag2
+EOT
+
+ 200 => <<-EOT,
+NAME DESCRIPTION AUTHOR KEYWORDS
+Name This description is really too long to fit in a single data table, guys -- we should probably set about trunca... @JohnnyApples tag1 tag2 taggitty3
+EOT
+ }.each do |width, expectation|
+ it "should resize the table to fit the screen, when #{width} columns" do
+ results = [
+ {
+ 'full_name' => 'Name',
+ 'author' => 'JohnnyApples',
+ 'desc' => 'This description is really too long to fit in a single data table, guys -- we should probably set about truncating it',
+ 'tag_list' => ['tag1', 'tag2', 'taggitty3'],
+ },
+ ]
+
+ Puppet::Util::Terminal.expects(:width).returns(width)
+ result = subject.render(results, ['apache', {}])
+ result.lines.sort_by(&:length).last.chomp.length.should <= width
+ result.should == expectation
+ end
+ end
+ end
+
describe "option validation" do
context "without any options" do
it "should require a search term" do
diff --git a/spec/unit/util/terminal_spec.rb b/spec/unit/util/terminal_spec.rb
new file mode 100644
index 0000000..d017549
--- /dev/null
+++ b/spec/unit/util/terminal_spec.rb
@@ -0,0 +1,34 @@
+#!/usr/bin/env rspec
+require 'spec_helper'
+require 'puppet/util/terminal'
+
+describe Puppet::Util::Terminal do
+ describe '.width' do
+ it 'should invoke `stty` and return the width' do
+ height, width = 100, 200
+ subject.expects(:`).with('stty size 2>/dev/null').returns("#{height} #{width}\n")
+ subject.width.should == width
+ end
+
+ it 'should use `tput` if `stty` is unavailable' do
+ width = 200
+ subject.expects(:`).with('stty size 2>/dev/null').returns("\n")
+ subject.expects(:`).with('tput cols 2>/dev/null').returns("#{width}\n")
+ subject.width.should == width
+ end
+
+ it 'should default to 80 columns if `tput` and `stty` are unavailable' do
+ width = 80
+ subject.expects(:`).with('stty size 2>/dev/null').returns("\n")
+ subject.expects(:`).with('tput cols 2>/dev/null').returns("\n")
+ subject.width.should == width
+ end
+
+ it 'should default to 80 columns if `tput` or `stty` raise exceptions' do
+ width = 80
+ subject.expects(:`).with('stty size 2>/dev/null').raises()
+ subject.stubs(:`).with('tput cols 2>/dev/null').returns("#{width + 1000}\n")
+ subject.width.should == width
+ end
+ end
+end
-- 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.
