]> git.cworth.org Git - sup/commitdiff
Merge branch 'dont-canonicalize-email-addresses' into next
authorWilliam Morgan <wmorgan-sup@masanjin.net>
Wed, 25 Mar 2009 15:52:16 +0000 (08:52 -0700)
committerWilliam Morgan <wmorgan-sup@masanjin.net>
Wed, 25 Mar 2009 15:52:16 +0000 (08:52 -0700)
12 files changed:
bugs/issue-aae5ae6378afa9bd2a8e1b15d28ba7ccef867791.yaml
lib/sup.rb
lib/sup/account.rb
lib/sup/buffer.rb
lib/sup/contact.rb
lib/sup/index.rb
lib/sup/message.rb
lib/sup/modes/edit-message-mode.rb
lib/sup/modes/reply-mode.rb
lib/sup/modes/thread-view-mode.rb
lib/sup/person.rb
test/test_message.rb

index cd3820da3e481d82c284661f3561f81fff8603f0..83cc00a4a4d1d1560b858c8809c5cc22ea5f32c4 100644 (file)
@@ -5,8 +5,8 @@ type: :bugfix
 component: sup
 release: 
 reporter: William Morgan <wmorgan-sup@masanjin.net>
-status: :unstarted
-disposition: 
+status: :closed
+disposition: :fixed
 creation_time: 2008-05-19 23:42:25.910550 Z
 references: []
 
@@ -20,4 +20,8 @@ log_events:
   - William Morgan <wmorgan-sup@masanjin.net>
   - unassigned from release 0.6
   - ""
+- - 2008-11-22 16:31:27.450146 Z
+  - Nicolas Pouillard <nicolas.pouillard@gmail.com>
+  - closed with disposition fixed
+  - This mapping and the PersonManager are now removed.
 git_branch: 
index f855964151863f8a29bc18b43cb9e2952e894fda..6d9f12531232bf888f0036a7027c5a8fb0a2e11e 100644 (file)
@@ -53,7 +53,6 @@ module Redwood
   COLOR_FN   = File.join(BASE_DIR, "colors.yaml")
   SOURCE_FN  = File.join(BASE_DIR, "sources.yaml")
   LABEL_FN   = File.join(BASE_DIR, "labels.txt")
-  PERSON_FN  = File.join(BASE_DIR, "people.txt")
   CONTACT_FN = File.join(BASE_DIR, "contacts.txt")
   DRAFT_DIR  = File.join(BASE_DIR, "drafts")
   SENT_FN    = File.join(BASE_DIR, "sent.mbox")
@@ -115,7 +114,6 @@ module Redwood
   end
 
   def start
-    Redwood::PersonManager.new Redwood::PERSON_FN
     Redwood::SentManager.new Redwood::SENT_FN
     Redwood::ContactManager.new Redwood::CONTACT_FN
     Redwood::LabelManager.new Redwood::LABEL_FN
@@ -131,7 +129,6 @@ module Redwood
   def finish
     Redwood::LabelManager.save if Redwood::LabelManager.instantiated?
     Redwood::ContactManager.save if Redwood::ContactManager.instantiated?
-    Redwood::PersonManager.save if Redwood::PersonManager.instantiated?
     Redwood::BufferManager.deinstantiate! if Redwood::BufferManager.instantiated?
   end
 
index f8ac0fcaf672ace7e2b72e4e666e2b56d7c6fc19..6f86129ca5d8997b0572f38512ba84a691ee4a7c 100644 (file)
@@ -5,8 +5,8 @@ class Account < Person
 
   def initialize h
     raise ArgumentError, "no name for account" unless h[:name]
-    raise ArgumentError, "no name for email" unless h[:name]
-    super h[:name], h[:email], 0, true
+    raise ArgumentError, "no email for account" unless h[:email]
+    super h[:name], h[:email]
     @sendmail = h[:sendmail]
     @signature = h[:signature]
   end
@@ -42,7 +42,6 @@ class AccountManager
     hash[:alternates] ||= []
 
     a = Account.new hash
-    PersonManager.register a
     @accounts[a] = true
 
     if default
index 2143debd66166a571734d20cb927d3984ff42b08..c9e34b3595c62b82b64350163e0d547e127f95eb 100644 (file)
@@ -506,7 +506,7 @@ EOS
     answer = BufferManager.ask_many_emails_with_completions domain, question, completions, default
 
     if answer
-      answer.split_on_commas.map { |x| ContactManager.contact_for(x) || PersonManager.person_for(x) }
+      answer.split_on_commas.map { |x| ContactManager.contact_for(x) || Person.from_address(x) }
     end
   end
 
index b0c272e8ece056b825ede520312f49d35de7d23c..51fd0e978c902f17485c8fcd81f20cc766b74157 100644 (file)
@@ -17,7 +17,7 @@ class ContactManager
       IO.foreach(fn) do |l|
         l =~ /^([^:]*): (.*)$/ or raise "can't parse #{fn} line #{l.inspect}"
         aalias, addr = $1, $2
-        p = PersonManager.person_for addr, :definitive => true
+        p = Person.from_address addr
         @p2a[p] = aalias
         @a2p[aalias] = p unless aalias.nil? || aalias.empty?
       end
index 8af4edd89e9a3c04799ac3d7a6774097d52d1630..838d601347f1f86ffba21ac72e37c3e191ac95ba 100644 (file)
@@ -246,8 +246,17 @@ EOS
       :snippet => snippet, # always override
       :label => labels.uniq.join(" "),
       :attachments => (entry[:attachments] || m.attachments.uniq.join(" ")),
-      :from => (entry[:from] || (m.from ? m.from.indexable_content : "")),
-      :to => (entry[:to] || (m.to + m.cc + m.bcc).map { |x| x.indexable_content }.join(" ")),
+
+      ## always override :from and :to.
+      ## older versions of Sup would often store the wrong thing in the index
+      ## (because they were canonicalizing email addresses, resulting in the
+      ## wrong name associated with each.) the correct address is read from
+      ## the original header when these messages are opened in thread-view-mode,
+      ## so this allows people to forcibly update the address in the index by
+      ## marking those threads for saving.
+      :from => (m.from ? m.from.indexable_content : ""),
+      :to => (m.to + m.cc + m.bcc).map { |x| x.indexable_content }.join(" "),
+
       :subject => (entry[:subject] || wrap_subj(Message.normalize_subj(m.subj))),
       :refs => (entry[:refs] || (m.refs + m.replytos).uniq.join(" ")),
     }
@@ -449,9 +458,9 @@ EOS
         t = @index[docid][:to]
 
         if AccountManager.is_account_email? f
-          t.split(" ").each { |e| contacts[PersonManager.person_for(e)] = true }
+          t.split(" ").each { |e| contacts[Person.from_address(e)] = true }
         else
-          contacts[PersonManager.person_for(f)] = true
+          contacts[Person.from_address(f)] = true
         end
       end
     end
index 33d801c3a81a30b5c51e5e15457f7b401873ce35..0ee46fb25d7ecbb97807910c9fa0785337bccc2c 100644 (file)
@@ -78,10 +78,10 @@ class Message
     
     @from =
       if header["from"]
-        PersonManager.person_for header["from"]
+        Person.from_address header["from"]
       else
         fakename = "Sup Auto-generated Fake Sender <sup@fake.sender.example.com>"
-        PersonManager.person_for fakename
+        Person.from_address fakename
       end
 
     Redwood::log "faking message-id for message from #@from: #{id}" if fakeid
@@ -105,9 +105,9 @@ class Message
       end
 
     @subj = header.member?("subject") ? header["subject"].gsub(/\s+/, " ").gsub(/\s+$/, "") : DEFAULT_SUBJECT
-    @to = PersonManager.people_for header["to"]
-    @cc = PersonManager.people_for header["cc"]
-    @bcc = PersonManager.people_for header["bcc"]
+    @to = Person.from_address_list header["to"]
+    @cc = Person.from_address_list header["cc"]
+    @bcc = Person.from_address_list header["bcc"]
 
     ## before loading our full header from the source, we can actually
     ## have some extra refs set by the UI. (this happens when the user
@@ -117,10 +117,10 @@ class Message
     @refs = (@refs + refs).uniq
     @replytos = (header["in-reply-to"] || "").scan(/<(.+?)>/).map { |x| sanitize_message_id x.first }
 
-    @replyto = PersonManager.person_for header["reply-to"]
+    @replyto = Person.from_address header["reply-to"]
     @list_address =
       if header["list-post"]
-        @list_address = PersonManager.person_for header["list-post"].gsub(/^<mailto:|>$/, "")
+        @list_address = Person.from_address header["list-post"].gsub(/^<mailto:|>$/, "")
       else
         nil
       end
@@ -391,7 +391,7 @@ private
     elsif m.header.content_type == "message/rfc822"
       payload = RMail::Parser.read(m.body)
       from = payload.header.from.first
-      from_person = from ? PersonManager.person_for(from.format) : nil
+      from_person = from ? Person.from_address(from.format) : nil
       [Chunk::EnclosedMessage.new(from_person, payload.to_s)] +
         message_to_chunks(payload, encrypted)
     else
index cc6e7af4a482d46dfefd74f5034846c81035620d..31aa8972766c34d17ae8f9cda5379b5363af559f 100644 (file)
@@ -321,8 +321,8 @@ protected
 
     ## do whatever crypto transformation is necessary
     if @crypto_selector && @crypto_selector.val != :none
-      from_email = PersonManager.person_for(@header["From"]).email
-      to_email = [@header["To"], @header["Cc"], @header["Bcc"]].flatten.compact.map { |p| PersonManager.person_for(p).email }
+      from_email = Person.from_address(@header["From"]).email
+      to_email = [@header["To"], @header["Cc"], @header["Bcc"]].flatten.compact.map { |p| Person.from_address(p).email }
 
       m = CryptoManager.send @crypto_selector.val, from_email, to_email, m
     end
@@ -412,7 +412,7 @@ private
   end
 
   def sig_lines
-    p = PersonManager.person_for(@header["From"])
+    p = Person.from_address(@header["From"])
     from_email = p && p.email
 
     ## first run the hook
index 4e08e8e9158bead401d437d4933d1a3ed0c42b81..c1c542b373e30bb74061e0ddfe502eddb31f5bc2 100644 (file)
@@ -63,7 +63,7 @@ EOS
       if hook_reply_from
         hook_reply_from
       elsif @m.recipient_email && AccountManager.is_account_email?(@m.recipient_email)
-        PersonManager.person_for(@m.recipient_email)
+        Person.from_address(@m.recipient_email)
       elsif(b = (@m.to + @m.cc).find { |p| AccountManager.is_account? p })
         b
       else
index f4d4232073e0126eef807e87170611c6840f344d..f27f00d81ec28bb7563dac53e2ccfd406ae93e2f 100644 (file)
@@ -149,7 +149,7 @@ EOS
   def subscribe_to_list
     m = @message_lines[curpos] or return
     if m.list_subscribe && m.list_subscribe =~ /<mailto:(.*?)\?(subject=(.*?))>/
-      ComposeMode.spawn_nicely :from => AccountManager.account_for(m.recipient_email), :to => [PersonManager.person_for($1)], :subj => $3
+      ComposeMode.spawn_nicely :from => AccountManager.account_for(m.recipient_email), :to => [Person.from_address($1)], :subj => $3
     else
       BufferManager.flash "Can't find List-Subscribe header for this message."
     end
@@ -158,7 +158,7 @@ EOS
   def unsubscribe_from_list
     m = @message_lines[curpos] or return
     if m.list_unsubscribe && m.list_unsubscribe =~ /<mailto:(.*?)\?(subject=(.*?))>/
-      ComposeMode.spawn_nicely :from => AccountManager.account_for(m.recipient_email), :to => [PersonManager.person_for($1)], :subj => $3
+      ComposeMode.spawn_nicely :from => AccountManager.account_for(m.recipient_email), :to => [Person.from_address($1)], :subj => $3
     else
       BufferManager.flash "Can't find List-Unsubscribe header for this message."
     end
index fb58f23889d53503bce539407b2583767b40a49c..c4f40a523e77907590d6eb1e546a37905fef6576 100644 (file)
@@ -1,62 +1,9 @@
 module Redwood
 
-class PersonManager
-  include Singleton
-
-  def initialize fn
-    @fn = fn
-    @@people = {}
-
-    ## read in stored people
-    IO.readlines(fn).map do |l|
-      l =~ /^(.*)?:\s+(\d+)\s+(.*)$/ or next
-      email, time, name = $1, $2, $3
-      @@people[email] = Person.new name, email, time, false
-    end if File.exists? fn
-
-    self.class.i_am_the_instance self
-  end
-
-  def save
-    File.open(@fn, "w") do |f|
-      @@people.each do |email, p|
-        next if p.email == p.name
-        next if p.name =~ /=/ # drop rfc2047-encoded, and lots of other useless emails. definitely a heuristic.
-        f.puts "#{p.email}: #{p.timestamp} #{p.name}"
-      end
-    end
-  end
-
-  def self.people_for s, opts={}
-    return [] if s.nil?
-    s.split_on_commas.map { |ss| self.person_for ss, opts }
-  end
-
-  def self.person_for s, opts={}
-    p = Person.from_address(s) or return nil
-    p.definitive = true if opts[:definitive]
-    register p
-  end
-  
-  def self.register p
-    oldp = @@people[p.email]
-
-    if oldp.nil? || p.better_than?(oldp)
-      @@people[p.email] = p
-    end
-
-    @@people[p.email].touch!
-    @@people[p.email]
-  end
-end
-
-## don't create these by hand. rather, go through personmanager, to
-## ensure uniqueness and overriding.
 class Person 
-  attr_accessor :name, :email, :timestamp
-  bool_accessor :definitive
+  attr_accessor :name, :email
 
-  def initialize name, email, timestamp=0, definitive=false
+  def initialize name, email
     raise ArgumentError, "email can't be nil" unless email
     
     if name
@@ -67,26 +14,10 @@ class Person
     end
 
     @email = email.gsub(/^\s+|\s+$/, "").gsub(/\s+/, " ").downcase
-    @definitive = definitive
-    @timestamp = timestamp
-  end
-
-  ## heuristic: whether the name attached to this email is "real", i.e. 
-  ## we should bother to store it.
-  def generic?
-    @email =~ /no\-?reply/
-  end
-
-  def better_than? o
-    return false if o.definitive? || generic?
-    return true if definitive?
-    o.name.nil? || (name && name.length > o.name.length && name =~ /[a-z]/)
   end
 
   def to_s; "#@name <#@email>" end
 
-  def touch!; @timestamp = Time.now.to_i end
-
 #   def == o; o && o.email == email; end
 #   alias :eql? :==
 #   def hash; [name, email].hash; end
@@ -146,8 +77,20 @@ class Person
     return nil if s.nil?
 
     ## try and parse an email address and name
-    name, email =
-      case s
+    name, email = case s
+      when /(.+?) ((\S+?)@\S+) \3/
+        ## ok, this first match cause is insane, but bear with me.  email
+        ## addresses are stored in the to/from/etc fields of the index in a
+        ## weird format: "name address first-part-of-address", i.e.  spaces
+        ## separating those three bits, and no <>'s. this is the output of
+        ## #indexable_content. here, we reverse-engineer that format to extract
+        ## a valid address.
+        ##
+        ## we store things this way to allow searches on a to/from/etc field to
+        ## match any of those parts. a more robust solution would be to store a
+        ## separate, non-indexed field with the proper headers. but this way we
+        ## save precious bits, and it's backwards-compatible with older indexes.
+        [$1, $2]
       when /["'](.*?)["'] <(.*?)>/, /([^,]+) <(.*?)>/
         a, b = $1, $2
         [a.gsub('\"', '"'), b]
@@ -162,6 +105,12 @@ class Person
     Person.new name, email
   end
 
+  def self.from_address_list ss
+    return [] if ss.nil?
+    ss.split_on_commas.map { |s| self.from_address s }
+  end
+
+  ## see comments in self.from_address
   def indexable_content
     [name, email, email.split(/@/).first].join(" ")
   end
index 96562aa7f53a3780c37cf91c17eb2c217daa5f4c..e38ac5064ca999f8b95b6d70c93f9b05ed8796f4 100644 (file)
@@ -29,11 +29,6 @@ module Redwood
 class TestMessage < Test::Unit::TestCase
   
   def setup
-    person_file = StringIO.new("")
-    # this is a singleton
-    if not PersonManager.instantiated?
-      @person_manager = PersonManager.new(person_file)
-    end
   end
   
   def teardown