Re: [sup-devel] Patch: Always try to canonize person name using ContactManager

From: Tero Tilus <tero@tilus.net>
To: Sup developers <sup-devel@rubyforge.org>
Subject: Re: [sup-devel] Patch: Always try to canonize person name using ContactManager
Date: Tue, 25 Jan 2011 23:25:03 +0200	[thread overview]
Message-ID: <1295990180-sup-2106@tilus.net> (raw)
In-Reply-To: <1295979620-sup-7844@alvh.no-ip.org>

[-- Attachment #1: Type: text/plain, Size: 997 bytes --]

Alvaro Herrera, 2011-01-25 20:29:
> One minor nit, probably material for another patch: the list of
> names that's built in the thread index mode doesn't seem to use this
> facility

When message was built from index it bypassed person canonization.
Amended patch attached.

> and so when you have a thread that includes several emails from the
> same account but inconsistent names, that person is listed more than
> once in the "participants" column, with the different names.
> 
> (For example I have a thread from Fabian, Fabián, and
> fabian.martinez that all come from the same
> fabian.martinez@example.com account).

Actually, this was completely independent of the person canonization
feature.  Thread index mode, when constructing the from-list,
identified authors by name, not by email.  The people not in
contacts.txt and appearing in from fields with varying names were
listed several times.

Attached patch also fixes this.

-- 
Tero Tilus ## 050 3635 235 ## http://tero.tilus.net/

[-- Attachment #2: 0002-Always-try-to-canonize-person-using-ContactManager.patch --]
[-- Type: application/octet-stream, Size: 4177 bytes --]

From b6afc55cd0539a38ef16b62c206181ef297c98df Mon Sep 17 00:00:00 2001
From: Tero Tilus <tero@tilus.net>
Date: Tue, 25 Jan 2011 01:12:30 +0200
Subject: [PATCH] Always try to canonize person using ContactManager

Signed-off-by: Tero Tilus <tero@tilus.net>
---
 lib/sup/contact.rb                 |   13 +++++++++----
 lib/sup/index.rb                   |    6 ++++--
 lib/sup/modes/thread-index-mode.rb |    4 ++--
 lib/sup/person.rb                  |    8 +++++++-
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/lib/sup/contact.rb b/lib/sup/contact.rb
index c489aaf..382896d 100644
--- a/lib/sup/contact.rb
+++ b/lib/sup/contact.rb
@@ -12,14 +12,13 @@ class ContactManager
 
     @p2a = {} # person to alias
     @a2p = {} # alias to person
+    @e2p = {} # email to person
 
     if File.exists? fn
       IO.foreach(fn) do |l|
         l =~ /^([^:]*): (.*)$/ or raise "can't parse #{fn} line #{l.inspect}"
         aalias, addr = $1, $2
-        p = Person.from_address addr
-        @p2a[p] = aalias
-        @a2p[aalias] = p unless aalias.nil? || aalias.empty?
+        update_alias Person.from_address(addr), aalias
       end
     end
   end
@@ -30,9 +29,13 @@ class ContactManager
   def update_alias person, aalias=nil
     if(old_aalias = @p2a[person]) # remove old alias
       @a2p.delete old_aalias
+      @e2p.delete old_aalias.email
     end
     @p2a[person] = aalias
-    @a2p[aalias] = person unless aalias.nil? || aalias.empty?
+    unless aalias.nil? || aalias.empty?
+      @a2p[aalias] = person
+      @e2p[person.email] = person
+    end
   end
 
   ## this may not actually be called anywhere, since we still keep contacts
@@ -40,11 +43,13 @@ class ContactManager
   def drop_contact person
     aalias = @p2a[person]
     @p2a.delete person
+    @e2p.delete person.email
     @a2p.delete aalias if aalias
   end
 
   def contact_for aalias; @a2p[aalias] end
   def alias_for person; @p2a[person] end
+  def person_for email; @e2p[email] end
   def is_aliased_contact? person; !@p2a[person].nil? end
 
   def save
diff --git a/lib/sup/index.rb b/lib/sup/index.rb
index bcc449b..95f104a 100644
--- a/lib/sup/index.rb
+++ b/lib/sup/index.rb
@@ -210,7 +210,9 @@ EOS
                     :labels => entry[:labels],
                     :snippet => entry[:snippet]
 
-    mk_person = lambda { |x| Person.new(*x.reverse!) }
+    # Try to find person from contacts before falling back to
+    # generating it from the address.
+    mk_person = lambda { |x| Person.from_name_and_email(*x.reverse!) }
     entry[:from] = mk_person[entry[:from]]
     entry[:to].map!(&mk_person)
     entry[:cc].map!(&mk_person)
@@ -235,7 +237,7 @@ EOS
       m = b.call
       ([m.from]+m.to+m.cc+m.bcc).compact.each { |p| contacts << [p.name, p.email] }
     end
-    contacts.to_a.compact.map { |n,e| Person.new n, e }[0...num]
+    contacts.to_a.compact[0...num].map { |n,e| Person.from_name_and_email n, e }
   end
 
   ## Yield each message-id matching query
diff --git a/lib/sup/modes/thread-index-mode.rb b/lib/sup/modes/thread-index-mode.rb
index 11548c7..040dfa5 100644
--- a/lib/sup/modes/thread-index-mode.rb
+++ b/lib/sup/modes/thread-index-mode.rb
@@ -792,8 +792,8 @@ protected
     authors = t.map do |m, *o|
       next unless m && m.from
       new[m.from] ||= m.has_label?(:unread)
-      next if seen[m.from.mediumname]
-      seen[m.from.mediumname] = true
+      next if seen[m.from]
+      seen[m.from] = true
       m.from
     end.compact
 
diff --git a/lib/sup/person.rb b/lib/sup/person.rb
index 28887b3..41a9a90 100644
--- a/lib/sup/person.rb
+++ b/lib/sup/person.rb
@@ -71,6 +71,12 @@ class Person
     end.downcase
   end
 
+  ## return "canonical" person using contact manager or create one if
+  ## not found or contact manager not available
+  def self.from_name_and_email name, email
+    ContactManager.instantiated? && ContactManager.person_for(email) || Person.new(name, email)
+  end
+
   def self.from_address s
     return nil if s.nil?
 
@@ -100,7 +106,7 @@ class Person
         [nil, s]
       end
 
-    Person.new name, email
+    from_name_and_email name, email
   end
 
   def self.from_address_list ss
-- 
1.5.6.5


[-- Attachment #3: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel
