Please review pull request #442: Tickets/2.7.x/12339 module search formatting opened by (pvande)
Description:
This aims to ensure that the most relevant data is always available in your
search results. The module name and author are always available, the most
relevant keywords are presented, and as much of the short description as
possible is shown.
- Opened: Thu Feb 02 00:08:11 UTC 2012
- Based on: puppetlabs:2.7.x (7485aa6e808121c876b5c9df477cc97183d4e535)
- Requested merge: pvande:tickets/2.7.x/12339-module-search-formatting (a7bd5698ab9bc8c11885737da2e7ec380275f24b)
Diff follows:
diff --git a/lib/puppet/face/module/search.rb b/lib/puppet/face/module/search.rb
index cec8d90..a1bee8c 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] }]
+ 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.max_by { |tl| tl.join(' ').length }
+ break if [ [], [term] ].include? longest_tag_list
+ longest_tag_list.delete(longest_tag_list.max_by { |t| t == term ? -1 : t.length })
+ 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..8674a49
--- /dev/null
+++ b/lib/puppet/util/terminal.rb
@@ -0,0 +1,15 @@
+module Puppet::Util::Terminal
+ class << self
+ # Borrowed shamelessly from Thor, who borrowed this from Rake.
+ def 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
+end
diff --git a/spec/unit/face/module/search_spec.rb b/spec/unit/face/module/search_spec.rb
index 250c5e0..974f49d 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 i... @Author tag1 tag2 taggitty3
+EOT
+
+ 70 => <<-EOT,
+NAME DESCRIPTION AUTHOR KEYWORDS
+Name This description is real... @Author tag1 tag2
+EOT
+
+ 80 => <<-EOT,
+NAME DESCRIPTION AUTHOR KEYWORDS
+Name This description is really too ... @Author 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 truncat... @Author 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' => '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', 'taggitty3'],
+ },
+ ]
+
+ Puppet::Util::Terminal.expects(:width).returns(width)
+ result = subject.render(results, ['apache', {}])
+ result.lines.max_by(&:length).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 puppet-dev@googlegroups.com.
To unsubscribe from this group, send email to puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
