Hi Edward,
Thanks for the patch. I'm interested in integrating it into Sup
mainline. A couple comments:
1. Can you rename it to custom-search? I think that's a better
description.
2. I would rather have the hook return a value. So this:
> - subs = s.gsub(/\b(to|from):(\S+)\b/) do
> + subs = String.new s
> + HookManager.run("before-search", :subs => subs)
I would rather see as:
subs = HookManager.run("before-search", :subs => s) || s
3. It's not really an issue of mutation vs no mutation. The problem is
that the parameter names in hooks are method calls, not variables. So
while "subs = subs.gsub(...)" causes a 'subs' local variable to be
created and initialized to nil, both "x = subs.gsub(...)" and "subs =
self.subs.gsub(...)" work fine.
This is probably less of an issue when the hook is returning a
values, but if you could change the comments to be a warning about
shadowing method calls instead that would be better.
Thanks!
--
William <[email protected]>
_______________________________________________
sup-talk mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/sup-talk
