On Tue, Jan 18, 2011 at 10:41 AM, Nick Lewis <[email protected]> wrote: > I get an error as soon as I try to run this: > /Users/nicklewis/puppet/puppet/lib/puppet/defaults.rb:104: undefined local > variable or method `diff' for Puppet:Module (NameError) > The Ruby interpreter is interpreting :show-diff as :show - diff. A possible > fix would be to use :"show-diff", if this is what's really intended. > However, I don't have enough context from the ticket or commit message to > understand the reason for this change. We have no options containing dashes, > and many containing underscores, so I question what this change is making > show_diff more consistent with. The only place we have dashes is in the > command-line form of specifying boolean options, in which case we use > --[no-]OPTION. In the actual option names, we are inconsistent between > underscores (show_diff, config_version) and no separator at all > (usecacheonfailure, downcasefacts). We do have an item on our technical debt > list for standardizing to using hyphens in our option names (at least on the > command line), but that will require a more general patch.
--detailed-exit-codes doesn't count? I jumped the gun and assumed we'd already had to solve this due to other options containing hyphens. > Nick > On Sun, Jan 16, 2011 at 4:28 PM, James Turnbull <[email protected]> > wrote: >> >> Signed-off-by: James Turnbull <[email protected]> >> --- >> lib/puppet/application/agent.rb | 4 ++-- >> lib/puppet/application/apply.rb | 2 +- >> lib/puppet/defaults.rb | 2 +- >> lib/puppet/type/file/content.rb | 2 +- >> spec/unit/application/apply_spec.rb | 4 ++-- >> spec/unit/type/file/content_spec.rb | 4 ++-- >> 6 files changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/lib/puppet/application/agent.rb >> b/lib/puppet/application/agent.rb >> index 2b75505..291e68e 100644 >> --- a/lib/puppet/application/agent.rb >> +++ b/lib/puppet/application/agent.rb >> @@ -137,7 +137,7 @@ class Puppet::Application::Agent < Puppet::Application >> Puppet.settings.handlearg("--ignorecache") >> Puppet.settings.handlearg("--no-usecacheonfailure") >> Puppet.settings.handlearg("--no-splay") >> - Puppet.settings.handlearg("--show_diff") >> + Puppet.settings.handlearg("--show-diff") >> Puppet.settings.handlearg("--no-daemonize") >> options[:verbose] = true >> Puppet[:onetime] = true >> @@ -202,7 +202,7 @@ class Puppet::Application::Agent < Puppet::Application >> exit(Puppet.settings.print_configs ? 0 : 1) if >> Puppet.settings.print_configs? >> >> # If noop is set, then also enable diffs >> - Puppet[:show_diff] = true if Puppet[:noop] >> + Puppet[:show-diff] = true if Puppet[:noop] >> >> args[:Server] = Puppet[:server] >> if options[:fqdn] >> diff --git a/lib/puppet/application/apply.rb >> b/lib/puppet/application/apply.rb >> index 33a70ce..af1d08c 100644 >> --- a/lib/puppet/application/apply.rb >> +++ b/lib/puppet/application/apply.rb >> @@ -137,7 +137,7 @@ class Puppet::Application::Apply < Puppet::Application >> exit(Puppet.settings.print_configs ? 0 : 1) if >> Puppet.settings.print_configs? >> >> # If noop is set, then also enable diffs >> - Puppet[:show_diff] = true if Puppet[:noop] >> + Puppet[:show-diff] = true if Puppet[:noop] >> >> Puppet::Util::Log.newdestination(:console) unless options[:logset] >> client = nil >> diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb >> index 4521a59..ad82205 100644 >> --- a/lib/puppet/defaults.rb >> +++ b/lib/puppet/defaults.rb >> @@ -101,7 +101,7 @@ module Puppet >> }, >> :diff_args => ["-u", "Which arguments to pass to the diff command when >> printing differences between files."], >> :diff => ["diff", "Which diff command to use when printing differences >> between files."], >> - :show_diff => [false, "Whether to print a contextual diff when files >> are being replaced. The diff >> + :show-diff => [false, "Whether to print a contextual diff when files >> are being replaced. The diff >> is printed on stdout, so this option is meaningless unless you are >> running Puppet interactively. >> This feature currently requires the `diff/lcs` Ruby library."], >> :daemonize => { :default => true, >> diff --git a/lib/puppet/type/file/content.rb >> b/lib/puppet/type/file/content.rb >> index b8f30a9..2620bab 100755 >> --- a/lib/puppet/type/file/content.rb >> +++ b/lib/puppet/type/file/content.rb >> @@ -100,7 +100,7 @@ module Puppet >> >> result = super >> >> - if ! result and Puppet[:show_diff] >> + if ! result and Puppet[:show-diff] >> write_temporarily do |path| >> print diff(@resource[:path], path) >> end >> diff --git a/spec/unit/application/apply_spec.rb >> b/spec/unit/application/apply_spec.rb >> index 4e17442..c70e857 100755 >> --- a/spec/unit/application/apply_spec.rb >> +++ b/spec/unit/application/apply_spec.rb >> @@ -61,12 +61,12 @@ describe Puppet::Application::Apply do >> @apply.options.stubs(:[]).with(any_parameters) >> end >> >> - it "should set show_diff on --noop" do >> + it "should set show-diff on --noop" do >> Puppet.stubs(:[]=) >> Puppet.stubs(:[]).with(:config) >> Puppet.stubs(:[]).with(:noop).returns(true) >> >> - Puppet.expects(:[]=).with(:show_diff, true) >> + Puppet.expects(:[]=).with(:show-diff, true) >> >> @apply.setup >> end >> diff --git a/spec/unit/type/file/content_spec.rb >> b/spec/unit/type/file/content_spec.rb >> index cde643f..b48d57e 100755 >> --- a/spec/unit/type/file/content_spec.rb >> +++ b/spec/unit/type/file/content_spec.rb >> @@ -169,9 +169,9 @@ describe content do >> @content.must be_insync("{md5}" + Digest::MD5.hexdigest("some >> content")) >> end >> >> - describe "and Puppet[:show_diff] is set" do >> + describe "and Puppet[:show-diff] is set" do >> before do >> - Puppet[:show_diff] = true >> + Puppet[:show-diff] = true >> end >> >> it "should display a diff if the current contents are different >> from the desired content" do >> -- >> 1.7.3.4 >> >> -- >> 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. >> > > -- > 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. > -- 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.
