Subject: [sup-talk] [PATCH] before-search hook

From: [email-masked] (Edward Z. Yang)
Date: Wed, 10 Jun 2009 01:44:54 -0400

>From a5c72844e8195df7a8eabd5ea592507599de72cb Mon Sep 17 00:00:00 2001
From: Edward Z. Yang <edwardzyang at thewritingpot.com>
Date: Wed, 10 Jun 2009 01:42:50 -0400
Subject: [PATCH] Add before-search hook, for shortcuts for custom search queries.
 Signed-off-by: Edward Z. Yang <edwardzyang at thewritingpot.com>

---
 lib/sup/hook.rb  |    4 ++++
 lib/sup/index.rb |   12 +++++++++++-
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/lib/sup/hook.rb b/lib/sup/hook.rb
index 0a0a2f6..d446aa3 100644
--- a/lib/sup/hook.rb
+++ b/lib/sup/hook.rb
@@ -19,6 +19,10 @@ class HookManager
 
     attr_writer :__locals
 
+    ## an annoying gotcha here is that if you try something
+    ## like var = var.foo(), var will magically get allocated
+    ## to Nil and method_missing will never get called.  Stick
+    ## to mutation, kiddos!
     def method_missing m, *a
       case @__locals[m]
       when Proc
diff --git a/lib/sup/index.rb b/lib/sup/index.rb
index e403570..19556f0 100644
--- a/lib/sup/index.rb
+++ b/lib/sup/index.rb
@@ -25,6 +25,13 @@ class Index
 
   include Singleton
 
+  HookManager.register "before-search", <<EOS
+Executes before a string search is applied to the index.
+Variables:
+  subs: The string being searched, use gsub! to change the
+  search (you must use mutation!)
+EOS
+
   ## these two accessors should ONLY be used by single-threaded programs.
   ## otherwise you will have a naughty ferret on your hands.
   attr_reader :index
@@ -508,7 +515,10 @@ protected
   def parse_user_query_string s
     extraopts = {}
 
-    subs = s.gsub(/\b(to|from):(\S+)\b/) do
+    subs = String.new s
+    HookManager.run("before-search", :subs => subs)
+
+    subs = subs.gsub(/\b(to|from):(\S+)\b/) do
       field, name = $1, $2
       if(p = ContactManager.contact_for(name))
         [field, p.email]
-- 
1.6.0.4

---

From: [email-masked] (Edward Z. Yang)
Date: Wed, 10 Jun 2009 17:51:01 -0400

For the curious, here is the hook that I'm using with this:

subs.gsub!(/\bmy:unread\b/) do
    "is:unread !label:inbox"
end

Cheers,
Edward

---

From: [email-masked] (William Morgan)
Date: Mon, 15 Jun 2009 10:44:25 -0700

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.

> +    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 <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Edward Z. Yang)
Date: Fri, 26 Jun 2009 13:10:01 -0400

Done.

>From 19f9da29020f1dfa97a4e6d0e4866cec840cfddc Mon Sep 17 00:00:00 2001
From: Edward Z. Yang <edwardzyang at thewritingpot.com>
Date: Wed, 10 Jun 2009 01:42:50 -0400
Subject: [PATCH 1/2] Add custom-search hook, for shortcuts for custom search queries.
 Signed-off-by: Edward Z. Yang <ezyang at mit.edu>

---
 lib/sup/hook.rb  |    5 +++++
 lib/sup/index.rb |   14 +++++++++++++-
 2 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/lib/sup/hook.rb b/lib/sup/hook.rb
index 0a0a2f6..33a97b2 100644
--- a/lib/sup/hook.rb
+++ b/lib/sup/hook.rb
@@ -19,6 +19,11 @@ class HookManager
 
     attr_writer :__locals
 
+    ## an annoying gotcha here is that if you try something
+    ## like var = var.foo(), var will magically get allocated
+    ## to Nil and method_missing will never get called.  You
+    ## can work around this by calling self.var or simply
+    ## not assigning it to itself.
     def method_missing m, *a
       case @__locals[m]
       when Proc
diff --git a/lib/sup/index.rb b/lib/sup/index.rb
index ca01ee7..9c985d9 100644
--- a/lib/sup/index.rb
+++ b/lib/sup/index.rb
@@ -24,6 +24,16 @@ class Index
 
   include Singleton
 
+  HookManager.register "custom-search", <<EOS
+Executes before a string search is applied to the index,
+returning a new search string.
+Variables:
+  subs: The string being searched. Be careful about shadowing:
+    this variable is actually a method, so use a temporary variable
+    or explicitly call self.subs; the substitutions in index.rb
+    don't actually work.
+EOS
+
   ## these two accessors should ONLY be used by single-threaded programs.
   ## otherwise you will have a naughty ferret on your hands.
   attr_reader :index
@@ -507,7 +517,9 @@ protected
   def parse_user_query_string s
     extraopts = {}
 
-    subs = s.gsub(/\b(to|from):(\S+)\b/) do
+    subs = HookManager.run("custom-search", :subs => s) || s
+
+    subs = subs.gsub(/\b(to|from):(\S+)\b/) do
       field, name = $1, $2
       if(p = ContactManager.contact_for(name))
         [field, p.email]
-- 
1.6.3.2

---

From: [email-masked] (Edward Z. Yang)
Date: Mon, 27 Jul 2009 13:40:48 -0400

> Done.

What's the status on getting this patch into the tree?

Cheers,
Edward

---

From: [email-masked] (William Morgan)
Date: Mon, 27 Jul 2009 12:40:05 -0700

> What's the status on getting this patch into the tree?

Thanks for the ping. I've put this on branch custom-search-hook and
merged into next. Note that it only applies to Ferret for now. Now that
we've split into two indexes, we need to plan out how this hook is going
to work going forward, since different indexes have different query
languages. Probably the best option is to have the index type be passed
as an argument, and let the user special-case on that.
-- 
William <wmorgan-sup at masanjin.net>