NoMethodError returns Smalltalk class name for RC Classes #96

Merged
merged 8 commits into from Nov 17, 2011

Projects

None yet

3 participants

@timfel
Contributor
timfel commented Nov 6, 2011

NoMethodError: NoMethodError: undefined methodempty' for RcKeyValueDictionary`

it should read

NoMethodError: NoMethodError: undefined methodempty' for RCHash`

Owner
timfel commented Nov 6, 2011

Fixing this isn't possible when using __resolve_smalltalk_global to assign the class name. This is exactly the same as doing:

   >> SS = String
   >> SS.foo
   => NoMethodError: undefined method `foo' for String:Class

The name will still be from the original class. __resolve_smalltalk_global wasn't really meant to be used outside bootstrap, either. I have added expose_smalltalk_global_as which takes two Strings, a Smalltalk class name and a Ruby name. It will install the constant under the Ruby name in the current namespace (so you don't have to do the assignment), and it'll setup the Ruby namespace so it prints the desired name for the class.

Contributor
jc00ke commented Nov 6, 2011

Awesome, thanks!

Owner
timfel commented Nov 7, 2011

I've added a test for this behaviour.
@pbm, @monty, if you don't object to this additional method, I'll just merge it. It's new API, anyway, so it wouldn't break existing code.

Member
pbm commented Nov 7, 2011

I think the test should cover both the transient and persistent cases. So we need to test that a transient use of expose_smalltalk_global, followed by a commit does not persist the symbol. And that a persistent use of expose_smalltalk_global, followed by a commit does persist the symbol.

Owner
timfel commented Nov 17, 2011

The problem with this implementation is now, that once you persistently expose a smalltalk class, you cannot change its name anymore (or it is very hard). I've played with different ways to fix this, but couldn't come up with a good solution. Is this ok? /cc @jc00ke @pbm

Member
pbm commented Nov 17, 2011

Looks good for now.

Contributor
jc00ke commented Nov 17, 2011

Cool

@timfel timfel merged commit 392c709 into master Nov 17, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment