pat / riddle

Ruby Client API for Sphinx
MIT License
135 stars 67 forks source link

Preserving original text on snippets #70

Closed epidemian closed 11 years ago

epidemian commented 11 years ago

This issue is somewhat a continuation of #63

Basically, i think the Riddle::Query.snippets function should preserve the original text sent to it (except for adding the <span class="match"> tags of course).

For example, these couple of specs added to the spec/functional/escaping_spec.rb would not pass:

  def snippets_for(text)
    res = connection.query Riddle::Query.snippets(text, 'people', '')
    res.first['snippet']
  end

  [
    'email: john@example.com',
    'the files are on C:\\foo\\bar'
  ].each do |text|
    it 'preserves original text on snippets' do
      snippets_for(text).should == text
    end
  end

The rspec output:

Failures:

  1) SphinxQL escaping preserves original text on snippets
     Failure/Error: non_matching_snippet(text).should == text
       expected: "email: john@example.com"
            got: "email: john\\@example.com" (using ==)
     # ./spec/functional/escaping_spec.rb:28:in `block (3 levels) in <top (required)>'

  2) SphinxQL escaping preserves original text on snippets
     Failure/Error: non_matching_snippet(text).should == text
       expected: "the files are on C:\\foo\\bar"
            got: "the files are on C:foobar" (using ==)
     # ./spec/functional/escaping_spec.rb:28:in `block (3 levels) in <top (required)>'

I don't think the examples are too far fetched, and they could certainly be part of a user-generated field on a DB (and the "excerpts" functionality of ThinkingSphinx will forward whatever some ActiveRecord attribute has to the Riddle::Query.snippets function).

I think the root of the problem is treating the to-be-excerpted text as SphinxQL when the CALL SNIPPETS function will not care about special SphinxQL characters (it will treat the data just as normal text as far as i know).

I'd propose to distinguish where the strings are interpreted as normal SQL strings on Sphinx calls, and apply only the Mysql2::Client.escape function in those cases.

pat commented 11 years ago

A pull request with those specs and the relevant fix would certainly be appreciated.

pat commented 11 years ago

Closed by #71.