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.

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]<puppet-dev%[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.

Reply via email to