Hey Pat, Many apologies-- in the patch I gave you, I meant to write ThinkingSphinx.deltas_enabled? which is the accessor to the module variable; without it, you'll run into: NoMethodError: undefined method `deltas_enabled' for ThinkingSphinx:Module
I've rectified the mistake in a commit to my fork and added an optional argument (reindex_after) that allows you to tweak whether a reindex happens after the block is excuted. I've also added spec tests for suspended_delta, so I can assure you with better certainty that it works. Grab as much as you will. Again, sorry for the bad patch; I'll only be submitting ones accompanied by spec tests from now on and via git to spare you trouble. -- Minh P.S. commit is at http://github.com/tranminhh/thinking-sphinx/commit/dcb4fbf51429f9c884684ea9c61d48491df50f22 On Oct 15, 9:03 pm, Pat Allan <[EMAIL PROTECTED]> wrote: > Hi Minh > > Thanks for the code suggestions yet again! Just made the changes and > pushed it out - it's all smart stuff. And credit where credit's due - > James Healy was the one who wrote this feature, all I had to do was > merge the change in. > > Cheers > > -- > Pat > > On 16/10/2008, at 5:00 AM, Minh Tran wrote: > > > > > Hi Pat, > > > I'm glad suspended_delta() is available in the master branch now; I've > > been monkey patching to get something like it for a while, so thank > > you! > > > I looked at the source code, and it was simple and clean, but may I > > suggest some best practices? I'll list each separately and provide my > > rationale. Thank you ahead of time for your patience. The original > > source code is pasted below for your convenience. > > > === > > def suspended_delta(&block) > > ThinkingSphinx.deltas_enabled = false > > yield > > ThinkingSphinx.deltas_enabled = true > > self.index_delta > > end > > === > > > First, I think we should wrap the yield in a begin/ensure block so > > that if there is an error, deltas will be re-enabled before the error > > gets passed up the stack. This is potentially important because the > > error may be caught and handled intelligently (in a way that requires > > a re-index). > > > Second, I think we shouldn't assume that deltas were enabled already; > > above, the suspended_delta() code *always* sets deltas_enabled to true > > rather than returns it to its previous value. Eliminating this > > assumption doesn't make the code much more complex but will make the > > method more robust. > > > If you find any of this agreeable, I included a patch at the end of > > this post. > > > As always, Pat, thank you for your work on this! Hope you are doing > > well =). > > > Sincerely, > > Minh Tran > > > ===== > > > --- a/lib/thinking_sphinx/active_record/delta.rb > > +++ b/lib/thinking_sphinx/active_record/delta.rb > > @@ -28,10 +28,14 @@ module ThinkingSphinx > > # end > > # > > def suspended_delta(&block) > > + orig_deltas = ThinkingSphinx.deltas_enabled > > ThinkingSphinx.deltas_enabled = false > > - yield > > - ThinkingSphinx.deltas_enabled = true > > - self.index_delta > > + begin > > + yield > > + ensure > > + ThinkingSphinx.deltas_enabled = orig_deltas > > + self.index_delta > > + end > > end > > > # Build the delta index for the related model. This won't > > be called --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Thinking Sphinx" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/thinking-sphinx?hl=en -~----------~----~----~----~------~----~------~--~---
