Thanks for that Minh - and don't stress, a lot of forks have their own custom patches, so I'm usually cherry-picking changes anyway.
Really appreciate the fix, and the specs even moreso! Thanks -- Pat On 17/10/2008, at 2:39 PM, Minh Tran wrote: > > I realized that a commit link to my master branch isn't very useful > because I have merged other branches (related to my work). I'm afraid > you would have to cherry pick(?), or I'm sure you know what's best. > > Sorry again for the inconvenience Pat, and for being a neophyte with > git. > -- > Minh > > On Oct 17, 12:14 am, Minh Tran <[EMAIL PROTECTED]> wrote: >> 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 >> athttp://github.com/tranminhh/thinking-sphinx/commit/dcb4fbf51429f9c884 >> ... >> >> 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 -~----------~----~----~----~------~----~------~--~---
