I had forgotten that we are using class variables to enable/disable CLI options. *shudder*
I think supporting anything other than a boolean in the 'desc' value is unnecessary - just accept true/false, and all that code gets a good bit simpler. Also, I think this is a bit strange when it comes to actually showing the descriptions, because the resolutions return a value but print the docs, which means (I think) you'll get a big list of docs and then in a completely separate set, the list of values (if you're printing more than one value). I'm frankly not sure how to solve this. Maybe the caller (which receives all of the values) should look up the docs when printing? On Mar 11, 2011, at 12:22 AM, James Turnbull wrote: > Facter.add(:factname) do > config :fact => value > desc "This describes the fact" > end > > The resulting descriptions are displayed: > > $ facter --desc > > Or: > > $ facter -D > > This is purely experimental - smarter people > than I can probably come up with a better way > of doing this. > > I've documented the id fact as an example. > > Signed-off-by: James Turnbull <[email protected]> > --- > Local-branch: tickets/master/5394 > bin/facter | 3 +++ > lib/facter.rb | 29 +++++++++++++++++++++++++++-- > lib/facter/application.rb | 1 + > lib/facter/id.rb | 2 ++ > lib/facter/util/resolution.rb | 8 ++++++++ > spec/unit/facter_spec.rb | 27 +++++++++++++++++++++++++++ > spec/unit/util/resolution_spec.rb | 6 +++++- > 7 files changed, 73 insertions(+), 3 deletions(-) > > diff --git a/bin/facter b/bin/facter > index 622d72e..f709787 100755 > --- a/bin/facter > +++ b/bin/facter > @@ -42,6 +42,9 @@ > # timing:: > # Enable timing. > # > +# desc:: > +# Enable fact descriptions > +# > # = Example > # > # facter kernel > diff --git a/lib/facter.rb b/lib/facter.rb > index f48138a..97fe173 100644 > --- a/lib/facter.rb > +++ b/lib/facter.rb > @@ -49,6 +49,7 @@ module Facter > RESET = " [0m" > @@debug = 0 > @@timing = 0 > + @@desc = 0 > > # module methods > > @@ -87,8 +88,15 @@ module Facter > @@timing != 0 > end > > - # Return a fact object by name. If you use this, you still have to call > - # 'value' on it to retrieve the actual value. > + # show the description > + def self.show_desc(desc) > + puts "#{GREEN}#{desc}#{RESET}" if desc and Facter.desc? > + end > + > + def self.desc? > + @@desc != 0 > + end > + > def self.[](name) > collection.fact(name) > end > @@ -204,6 +212,23 @@ module Facter > end > end > > + # Set desc on or off. > + def self.desc(bit) > + if bit > + case bit > + when TrueClass; @@desc = 1 > + when Fixnum > + if bit > 0 > + @@desc = 1 > + else > + @@desc = 0 > + end > + end > + else > + @@desc = 0 > + end > + end > + > def self.warn(msg) > if Facter.debugging? and msg and not msg.empty? > msg = [msg] unless msg.respond_to? :each > diff --git a/lib/facter/application.rb b/lib/facter/application.rb > index 6b351b1..8d63f78 100644 > --- a/lib/facter/application.rb > +++ b/lib/facter/application.rb > @@ -74,6 +74,7 @@ module Facter > opts.on( "--trace") { |v| options[:trace] = v } > opts.on("-d", "--debug") { |v| Facter.debugging(1) } > opts.on("-t", "--timing") { |v| Facter.timing(1) } > + opts.on("-D", "--desc") { |v| Facter.desc(1) } > opts.on("-p", "--puppet") { |v| load_puppet } > > opts.on_tail("-v", "--version") do > diff --git a/lib/facter/id.rb b/lib/facter/id.rb > index c2c3594..ec8b688 100644 > --- a/lib/facter/id.rb > +++ b/lib/facter/id.rb > @@ -1,10 +1,12 @@ > Facter.add(:id) do > confine :operatingsystem => %w{Linux Fedora RedHat CentOS SuSE SLES > Debian Ubuntu Gentoo AIX OEL OVS GNU/kFreeBSD windows} > + desc "The id fact uses whoami to return an id" > setcode "whoami" > end > > Facter.add(:id) do > confine :operatingsystem => %w{Solaris} > + desc "The id fact on Solaris uses the id binary to return an id" > setcode do > if %x{id} =~ /^uid=\d+\((\S+)\)/ > $1 > diff --git a/lib/facter/util/resolution.rb b/lib/facter/util/resolution.rb > index 4a99c35..7a9235f 100644 > --- a/lib/facter/util/resolution.rb > +++ b/lib/facter/util/resolution.rb > @@ -85,10 +85,16 @@ class Facter::Util::Resolution > end > end > > + # Add a new description to the resolution mechanism > + def desc(desc) > + @desc = desc > + end > + > # Create a new resolution mechanism. > def initialize(name) > @name = name > @confines = [] > + @desc = nil > @value = nil > @timeout = 0 > end > @@ -163,6 +169,8 @@ class Facter::Util::Resolution > ms = (finishtime - starttime) * 1000 > Facter.show_time "#{self.name}: #{"%.2f" % ms}ms" > > + Facter.show_desc "#{self.name}: %s" % @desc unless @desc.nil? > + > return nil if result == "" > return result > end > diff --git a/spec/unit/facter_spec.rb b/spec/unit/facter_spec.rb > index e63bc76..eef910a 100755 > --- a/spec/unit/facter_spec.rb > +++ b/spec/unit/facter_spec.rb > @@ -143,6 +143,14 @@ describe Facter do > Facter.should respond_to(:show_time) > end > > + it "should have a method to query description mode" do > + Facter.should respond_to(:desc?) > + end > + > + it "should have a method to show description information" do > + Facter.should respond_to(:show_desc) > + end > + > it "should have a method to warn" do > Facter.should respond_to(:warn) > end > @@ -231,6 +239,25 @@ describe Facter do > end > end > > + describe "when setting description mode" do > + it "should have descriptions enabled using 1" do > + Facter.desc(1) > + Facter.should be_desc > + end > + it "should have descriptions enabled using true" do > + Facter.desc(true) > + Facter.should be_desc > + end > + it "should have descriptions disabled using 0" do > + Facter.desc(0) > + Facter.should_not be_desc > + end > + it "should have descriptions disabled using false" do > + Facter.desc(false) > + Facter.should_not be_desc > + end > + end > + > describe "when registering directories to search" do > after { Facter.instance_variable_set("@search_path", []) } > > diff --git a/spec/unit/util/resolution_spec.rb > b/spec/unit/util/resolution_spec.rb > index 581d0e1..e6fb887 100755 > --- a/spec/unit/util/resolution_spec.rb > +++ b/spec/unit/util/resolution_spec.rb > @@ -17,6 +17,10 @@ describe Facter::Util::Resolution do > Facter::Util::Resolution.new("yay").should respond_to(:setcode) > end > > + it "should have a method for setting the description" do > + Facter::Util::Resolution.new("yay").should respond_to(:desc) > + end > + > it "should support a timeout value" do > Facter::Util::Resolution.new("yay").should respond_to(:timeout=) > end > @@ -93,7 +97,7 @@ describe Facter::Util::Resolution do > Facter::Util::Resolution::WINDOWS = true > Facter::Util::Resolution::INTERPRETER = "cmd.exe" > end > - > + > it "should return the result of executing the code with the > interpreter" do > @resolve.setcode "/bin/foo" > > Facter::Util::Resolution.expects(:exec).once.with("/bin/foo", > "cmd.exe").returns "yup" > -- > 1.7.1 > > -- > 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. > -- The ships hung in the sky in much the same way that bricks don't. -- Douglas Adams --------------------------------------------------------------------- Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199 -- 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.
