Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix superclass+dup mix

  • Loading branch information...
commit 58ef2cbd3c762cffb23200cb4e6741afb09a4f8d 1 parent de6b9a4
@josevalim josevalim authored
Showing with 25 additions and 2 deletions.
  1. +8 −1 lib/thor/base.rb
  2. +1 −1  lib/thor/version.rb
  3. +16 −0 spec/thor_spec.rb
View
9 lib/thor/base.rb
@@ -592,7 +592,14 @@ def from_superclass(method, default=nil)
default
else
value = superclass.send(method)
- value.dup if value
+
+ if value
+ if value.is_a?(TrueClass) || value.is_a?(Symbol)
+ value
+ else
+ value.dup
+ end
+ end
@sferik Owner
sferik added a note

IMHO, this would be clearer as a case statement:

case value
when TrueClass, FalseClass, NilClass, Symbol
  value
else
  value.dup
end
@eventualbuddha Collaborator

Why TrueClass et al? Why not simply this?

case value
when true, false, nil, Symbol
  value
else
  value.dup
end

Granted, I don't understand the problem this commit was trying to fix so I may be missing something.

@sferik Owner
sferik added a note

That would work too, given that TrueClass, FalseClass, and NilClass are all singletons. It's just a matter of style. I don't necessarily agree that your code is simpler than mine. In my example, all the comparisons are made against classes, while in your example, you compare value to both instances and classes. In my opinion, this has slightly higher cognitive overhead, but I'd prefer either of these implementations to the status quo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
end
end
View
2  lib/thor/version.rb
@@ -1,3 +1,3 @@
class Thor
- VERSION = "0.15.2"
+ VERSION = "0.15.3"
end
View
16 spec/thor_spec.rb
@@ -398,5 +398,21 @@ def unknown(*args)
klass.start(["unknown", "--bar", "baz"]).should == []
klass.start(["unknown", "foo", "--bar", "baz"]).should == ["foo"]
end
+
+ it "strict args works in the inheritance chain" do
+ parent = Class.new(Thor) do
+ strict_args_position!
+ end
+
+ klass = Class.new(parent) do
+ desc "unknown", "passing unknown options"
+ def unknown(*args)
+ args
+ end
+ end
+
+ klass.start(["unknown", "--bar", "baz"]).should == []
+ klass.start(["unknown", "foo", "--bar", "baz"]).should == ["foo"]
+ end
end
end
Please sign in to comment.
Something went wrong with that request. Please try again.