Skip to content

#159: Revise should(_not) to work with MacRuby #160

Merged
merged 1 commit into from Jul 25, 2012

5 participants

@ggilder
ggilder commented Jul 25, 2012

I believe this should fix issue #159. See build status here:

http://travis-ci.org/#!/ggilder/rspec-expectations/builds/1948099

@ggilder ggilder #159: Revise should(_not) to work with MacRuby 9c9619b
@travisbot

This pull request passes (merged 9c9619b into 024002c).

@myronmarston
RSpec member

This looks like a reasonable fix, but I'd like to understand the issue. Why does MacRuby have a BasicObject but not have it as the root of the object hierarchy?

@ggilder
ggilder commented Jul 25, 2012

I wish I had an answer for that. From my understanding, which is certainly limited, MacRuby includes BasicObject to maintain some compatibility with MRI, but has a different object hierarchy in order to bridge Ruby objects to their Cocoa equivalents. For instance, in MacRuby Object == NSObject. This allows you to mix Object and NSObject methods in MacRuby subclasses, but I think it makes it impossible to treat BasicObject in the same way as MRI because you can't modify NSObject's ancestor chain.

Hopefully that clarifies a bit... this hits the limits of my understanding of MacRuby's object model so we could bring it up on the MacRuby mailing list to get more information.

@alindeman

I saw this from a coworker:

MacRuby's classes implementation is based on Objective-C.
Ruby's Object class is alias of Objective-C's NSObject.

So, Ruby classes/methods and objects are Objective-C classes, methods
and objects respectively. The converse is also true.
Objective-C and MacRuby APIs can be interchangeable at no additional
performance expense.

NSObject is root class in Objective-C. NSObject doesn't have an ancestor.
However, CRuby's Object class has BasicObject as ancestor since CRuby1.9.

I think Objective-C classes does not have a class like Ruby's BasicClass.
And, Apple can only change the NSObject ancestor.

So, MacRuby's Object/BasicObject relationship is different from CRuby.

Edit: Just realized that it's related to this pull request, doh! Source: http://lists.macosforge.org/pipermail/macruby-devel/2012-July/008886.html

@myronmarston myronmarston merged commit d51bf72 into rspec:master Jul 25, 2012
@myronmarston
RSpec member

Thanks, that's helpful to understand it a bit more now.

@myronmarston myronmarston added a commit that referenced this pull request Jul 25, 2012
@myronmarston myronmarston Add change log entry for #160. 147f710
@alindeman alindeman commented on the diff Jul 25, 2012
lib/rspec/expectations/syntax.rb
@@ -40,8 +40,7 @@ module Syntax
# @api private
# Determines where we add `should` and `should_not`.
def default_should_host
- @default_should_host ||= defined?(::BasicObject) ?
- ::BasicObject : ::Kernel
+ @default_should_host ||= Object.ancestors.last
@alindeman
alindeman added a note Jul 25, 2012

Does it make sense to use ::Object here instead of Object? It'd be pretty crazy for there to be an RSpec::Object or RSpec::Expectations::Object, but 1) Ruby's constant resolution rules have changed and I don't always remember them and 2) We were originally defensive about this.

/cc: @myronmarston

@myronmarston
RSpec member
myronmarston added a note Jul 25, 2012

Probably a good idea--want to make that change?

@alindeman
alindeman added a note Jul 25, 2012

Yep! On it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@justinko

👏

@ggilder
ggilder commented Jul 25, 2012

Awesome, thanks all! 🎈🎈🎈

@alindeman

Does this warrant a patch release?

@myronmarston myronmarston added a commit that referenced this pull request Jul 26, 2012
@myronmarston myronmarston Add change log entry for #160. ca74dce
@myronmarston
RSpec member

Does this warrant a patch release?

Yep, I just released 2.11.2 with this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.