On Dec 17, 11:18 am, David Lee <[email protected]> wrote: > > Unfortunately, I don't think it's a good idea to include this into > > Sequel, as it modifies the number of arguments to the :eager_loader > > proc, which would break all custom eager loaders (and I don't like > > hacks like checking proc arity). There would have to be wide support > > for Sequel users for this patch's inclusion before I would consider > > it. > > I agree the ability to pass options to the eager loaded dataset on > runtime is a good idea. Is there a way to achieve this without > changing the number of arguments to :eager_loader? I'm thinking that > eager_loading_dataset can be instance_execed from post_load so that > calling_ds doesn't have to be passed in. In order to do this, the > default :eager_loader would also need to remove its dependency on the > model variable. Would this work?
Currently, instance_eval isn't used on the eager_loader proc. Using instance_exec isn't allowed because Sequel has to run on 1.8. Even if instance_eval could take arguments on 1.8, switching from a regular proc call to an instance_eval has the potential to break custom eager_loaders that were programmed with the assumption that the context inside the proc is the same as the context outside the proc (i.e. eager_loaders that call instance methods from the surrounding scope). It's not that I think this patch is a bad idea, it's that I don't think the breakage of backwards compatibility is worth it for the extremely rare case where it would be needed. Also, I didn't mention this earlier, but using eager_graph instead of eager would probably solve the issue, since then you could just apply arbitrary filters/orders to the dataset before calling all. Jeremy -- You received this message because you are subscribed to the Google Groups "sequel-talk" 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/sequel-talk?hl=en.
