Expected thinking_sphinx/search.rb to define ThinkingSphinx::Search #854

Closed
ghazel opened this Issue May 3, 2011 · 13 comments

Comments

Projects
None yet
5 participants
Contributor

ghazel commented May 3, 2011

I can't seem to start a rails app, due an error about thinking_sphinx/search.rb not defining ThinkingSphinx::Search, which it does. Very easily reproducible on a fresh Rails 2.3.11 app with config.gem 'thinking-sphinx' added to config/environment.rb

This happened after some recent changes in master (over the past week or so). Perhaps related to 50ddb62 but I'm not sure.

Owner

evanphx commented May 3, 2011

It's not related to that commit.

Please provide a gist of the backtrace you're getting and exact repro steps (assume I don't know how to make a rails 2.3.11 app).

Contributor

ghazel commented May 3, 2011

$ gem install rails -v 2.3.11
$ rails myapp
$ cd myapp
$ emacs config/environment.rb
... put "config.gem 'thinking-sphinx', :version => '~> 1.4.4'" after the comments about config.gem ...
$ gem install thinking-sphinx -v "~>1.4.4"
$ ruby script/console

https://gist.github.com/9d3fa09934966d07598a

Owner

evanphx commented May 3, 2011

This is because ThinkingSphinx monkeypatches Array.=== and that new method is called before Search is introduced. TS should not be changing Array.===, but I'm looking for a solution.

pat commented May 3, 2011

I don't like the hack either, but the reason it's in TS is so Rails treats a Search object as an Array (which, for all intents and purposes, it is). If anyone has a better way around this, I'll happily change TS to follow that.

Contributor

ghazel commented May 7, 2011

Why not subclass Array?

Contributor

ghazel commented May 7, 2011

The other answer I've seen is to include Enumerable, depending on which part of Rails you're talking about treating the Search instances as Arrays.

pat commented May 7, 2011

The reason I didn't subclass Array in the first place is because I wanted lazy loading of search results (for chaining scopes), and I hit a wall with that approach, so instead, I had an array within ThinkingSphinx::Search, and passed all array-related methods to it.

However, I didn't think of taking that second approach and making ThinkingSphinx::Search a subclass of Array - and once the monkeypatch is removed, it works!

Have gone through the test you've provided Greg (had to use a locally built gem, mind you – Bundler makes this less painful in Rails 3), all fine. However, the test doesn't fail with existing releases of TS, so best you check it out to make sure (grab the source, bundle, run rake build and you'll find the gem at pkg/thinking-sphinx-1.4.4.gem).

Contributor

ghazel commented May 7, 2011

Can't seem to bundle because Rubinius can't build rcov, and excluding that from the dependencies I can't rake build because "call/cc is not implemented!" (used by the "yard" gem).

Contributor

ghazel commented May 7, 2011

Anyway, I manually applied the patch since it didn't really require a recompile, and it worked.

Member

steveklabnik commented Oct 12, 2011

If it worked then this can be closed.

Contributor

ghazel commented Oct 12, 2011

Well, the patch to ThinkingSphinx worked. The ticket was still open because rbx's behavior differed from MRI here (see evanphx's comment)

Member

steveklabnik commented Oct 12, 2011

Ah, I misunderstood. I'll re-open then.

steveklabnik reopened this Oct 12, 2011

Owner

dbussink commented Mar 7, 2012

Also tested this with these old versions and seems to work now. Closing this one. If this issue still persists with new and up to date versions, please let us know!

dbussink closed this Mar 7, 2012

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