From: William Morgan <wmorgan-sup@masanjin.net>
Date: Tue, 11 Aug 2009 20:00:52 +0000 (-0400)
Subject: maintain labels as Sets rather than arrays
X-Git-Url: https://git.cworth.org/git?a=commitdiff_plain;h=7aea418a8a62b7070eee764475fcfc0bdd8d58dd;p=sup

maintain labels as Sets rather than arrays
---

diff --git a/bin/sup-dump b/bin/sup-dump
index ba36b21..992fd0b 100755
--- a/bin/sup-dump
+++ b/bin/sup-dump
@@ -26,5 +26,5 @@ Redwood::SourceManager.new
 index.load
 
 index.each_message :load_spam => true, :load_deleted => true, :load_killed => true do |m|
-  puts "#{m.id} (#{m.labels * ' '})"
+  puts "#{m.id} (#{m.labels.to_a * ' '})"
 end
diff --git a/bin/sup-sync b/bin/sup-sync
index 44ff3b2..f233072 100755
--- a/bin/sup-sync
+++ b/bin/sup-sync
@@ -102,7 +102,7 @@ restored_state =
     IO.foreach opts[:restore] do |l|
       l =~ /^(\S+) \((.*?)\)$/ or raise "Can't read dump line: #{l.inspect}"
       mid, labels = $1, $2
-      dump[mid] = labels.symbolistize
+      dump[mid] = labels.to_set_of_symbols
     end
     $stderr.puts "Read #{dump.size} entries from dump file."
     dump
@@ -159,19 +159,22 @@ begin
       index_state = m_old.labels.dup if m_old
 
       ## skip if we're operating on restored messages, and this one
-      ## ain't.
-      next if target == :restored && (!restored_state[m.id] || (index_state && restored_state[m.id].sort_by { |s| s.to_s } == index_state.sort_by { |s| s.to_s }))
+      ## ain't (or we wouldn't be making a change)
+      next if target == :restored && (!restored_state[m.id] || !index_state || restored_state[m.id] == index_state)
 
       ## m.labels is the default source labels. tweak these according
       ## to default source state modification flags.
-      m.labels -= [:inbox] if opts[:archive]
-      m.labels -= [:unread] if opts[:read]
-      m.labels += opts[:extra_labels].strip.split(/\s*,\s*/).map { |x| x.intern } if opts[:extra_labels]
+      m.labels.delete :inbox if opts[:archive]
+      m.labels.delete :unread if opts[:read]
+      m.labels += opts[:extra_labels].to_set_of_symbols(",") if opts[:extra_labels]
 
       ## assign message labels based on the operation we're performing
       case op
       when :asis
-        m.labels = ((m.labels - [:unread, :inbox]) + index_state).uniq if index_state
+        ## duplicate behavior of poll mode: if index_state is non-nil, this is a newer
+        ## version of an older message, so merge in any new labels except :unread and
+        ## :inbox.
+        m.labels = ((m.labels - [:unread, :inbox]) + index_state) if index_state
       when :restore
         ## if the entry exists on disk
         if restored_state[m.id]
@@ -193,10 +196,10 @@ begin
       end
 
       if index_state.nil?
-        puts "Adding message #{source}##{offset} from #{m.from} with state {#{m.labels * ', '}}" if opts[:verbose]
+        puts "Adding message #{source}##{offset} from #{m.from} with state {#{m.labels.to_a * ', '}}" if opts[:verbose]
         num_added += 1
       else
-        puts "Updating message #{source}##{offset}, source #{m_old.source.id} => #{source.id}, offset #{m_old.source_info} => #{offset}, state {#{index_state * ', '}} => {#{m.labels * ', '}}" if opts[:verbose]
+        puts "Updating message #{source}##{offset}, source #{m_old.source.id} => #{source.id}, offset #{m_old.source_info} => #{offset}, state {#{index_state.to_a * ', '}} => {#{m.labels.to_a * ', '}}" if opts[:verbose]
         num_updated += 1
       end
 
diff --git a/bin/sup-tweak-labels b/bin/sup-tweak-labels
index 8ae5c26..905aac2 100755
--- a/bin/sup-tweak-labels
+++ b/bin/sup-tweak-labels
@@ -54,8 +54,8 @@ EOS
 end
 opts[:verbose] = true if opts[:very_verbose]
 
-add_labels = (opts[:add] || "").split(",").map { |l| l.intern }.uniq
-remove_labels = (opts[:remove] || "").split(",").map { |l| l.intern }.uniq
+add_labels = opts[:add].to_set_of_symbols ","
+remove_labels = opts[:remove].to_set_of_symbols ","
 
 Trollop::die "nothing to do: no labels to add or remove" if add_labels.empty? && remove_labels.empty?
 
@@ -95,16 +95,15 @@ begin
     num_scanned += 1
 
     m = index.build_message id
-    old_labels = m.labels.clone
+    old_labels = m.labels.dup
 
     m.labels += add_labels
     m.labels -= remove_labels
-    m.labels = m.labels.uniq
 
-    unless m.labels.sort_by { |s| s.to_s } == old_labels.sort_by { |s| s.to_s }
+    unless m.labels == old_labels
       num_changed += 1
       puts "From #{m.from}, subject: #{m.subj}" if opts[:very_verbose]
-      puts "#{m.id}: {#{old_labels.join ','}} => {#{m.labels.join ','}}" if opts[:verbose]
+      puts "#{m.id}: {#{old_labels.to_a.join ','}} => {#{m.labels.to_a.join ','}}" if opts[:verbose]
       puts if opts[:very_verbose]
       index.sync_message m unless opts[:dry_run]
     end
diff --git a/lib/sup/buffer.rb b/lib/sup/buffer.rb
index 5f52d1d..3266473 100644
--- a/lib/sup/buffer.rb
+++ b/lib/sup/buffer.rb
@@ -495,7 +495,7 @@ EOS
 
     return unless answer
 
-    user_labels = answer.symbolistize
+    user_labels = answer.to_set_of_symbols
     user_labels.each do |l|
       if forbidden_labels.include?(l) || LabelManager::RESERVED_LABELS.include?(l)
         BufferManager.flash "'#{l}' is a reserved label!"
diff --git a/lib/sup/ferret_index.rb b/lib/sup/ferret_index.rb
index f3d4147..546faf8 100644
--- a/lib/sup/ferret_index.rb
+++ b/lib/sup/ferret_index.rb
@@ -79,13 +79,13 @@ class FerretIndex < BaseIndex
     ## written in this manner to support previous versions of the index which
     ## did not keep around the entry body. upgrading is thus seamless.
     entry ||= {}
-    labels = m.labels.uniq # override because this is the new state, unless...
+    labels = m.labels # override because this is the new state, unless...
 
     ## if we are a later version of a message, ignore what's in the index,
     ## but merge in the labels.
     if entry[:source_id] && entry[:source_info] && entry[:label] &&
       ((entry[:source_id].to_i > source_id) || (entry[:source_info].to_i < m.source_info))
-      labels = (entry[:label].symbolistize + m.labels).uniq
+      labels += entry[:label].to_set_of_symbols
       #Redwood::log "found updated version of message #{m.id}: #{m.subj}"
       #Redwood::log "previous version was at #{entry[:source_id].inspect}:#{entry[:source_info].inspect}, this version at #{source_id.inspect}:#{m.source_info.inspect}"
       #Redwood::log "merged labels are #{labels.inspect} (index #{entry[:label].inspect}, message #{m.labels.inspect})"
@@ -103,7 +103,7 @@ class FerretIndex < BaseIndex
       :date => (entry[:date] || m.date.to_indexable_s),
       :body => (entry[:body] || m.indexable_content),
       :snippet => snippet, # always override
-      :label => labels.uniq.join(" "),
+      :label => labels.to_a.join(" "),
       :attachments => (entry[:attachments] || m.attachments.uniq.join(" ")),
 
       ## always override :from and :to.
@@ -259,7 +259,7 @@ class FerretIndex < BaseIndex
       }
 
       m = Message.new :source => source, :source_info => doc[:source_info].to_i,
-                  :labels => doc[:label].symbolistize,
+                  :labels => doc[:label].to_set_of_symbols,
                   :snippet => doc[:snippet]
       m.parse_header fake_header
       m
diff --git a/lib/sup/imap.rb b/lib/sup/imap.rb
index 6c04d88..14acdb7 100644
--- a/lib/sup/imap.rb
+++ b/lib/sup/imap.rb
@@ -4,6 +4,7 @@ require 'stringio'
 require 'time'
 require 'rmail'
 require 'cgi'
+require 'set'
 
 ## TODO: remove synchronized method protector calls; use a Monitor instead
 ## (ruby's reentrant mutex)
@@ -69,7 +70,7 @@ class IMAP < Source
     @imap_state = {}
     @ids = []
     @last_scan = nil
-    @labels = ((labels || []) - LabelManager::RESERVED_LABELS).uniq.freeze
+    @labels = Set.new((labels || []) - LabelManager::RESERVED_LABELS)
     @say_id = nil
     @mutex = Mutex.new
   end
diff --git a/lib/sup/label.rb b/lib/sup/label.rb
index 47d632b..3e7bacb 100644
--- a/lib/sup/label.rb
+++ b/lib/sup/label.rb
@@ -61,9 +61,9 @@ class LabelManager
       l
     end
   end
-  
+
   def << t
-    t = t.intern unless t.is_a? Symbol
+    raise ArgumentError, "expecting a symbol" unless t.is_a? Symbol
     unless @labels.member?(t) || RESERVED_LABELS.member?(t)
       @labels[t] = true
       @new_labels[t] = true
diff --git a/lib/sup/maildir.rb b/lib/sup/maildir.rb
index c6577c1..80903e2 100644
--- a/lib/sup/maildir.rb
+++ b/lib/sup/maildir.rb
@@ -23,7 +23,7 @@ class Maildir < Source
     raise ArgumentError, "maildir URI must have a path component" unless uri.path
 
     @dir = uri.path
-    @labels = (labels || []).freeze
+    @labels = Set.new(labels || [])
     @ids = []
     @ids_to_fns = {}
     @last_scan = nil
diff --git a/lib/sup/mbox/loader.rb b/lib/sup/mbox/loader.rb
index ea277cf..f35c7df 100644
--- a/lib/sup/mbox/loader.rb
+++ b/lib/sup/mbox/loader.rb
@@ -1,17 +1,17 @@
 require 'rmail'
 require 'uri'
+require 'set'
 
 module Redwood
 module MBox
 
 class Loader < Source
   yaml_properties :uri, :cur_offset, :usual, :archived, :id, :labels
-  attr_accessor :labels
 
   ## uri_or_fp is horrific. need to refactor.
-  def initialize uri_or_fp, start_offset=0, usual=true, archived=false, id=nil, labels=[]
+  def initialize uri_or_fp, start_offset=0, usual=true, archived=false, id=nil, labels=nil
     @mutex = Mutex.new
-    @labels = ((labels || []) - LabelManager::RESERVED_LABELS).uniq.freeze
+    @labels = Set.new((labels || []) - LabelManager::RESERVED_LABELS)
 
     case uri_or_fp
     when String
@@ -168,7 +168,7 @@ class Loader < Source
     end
 
     self.cur_offset = next_offset
-    [returned_offset, (self.labels + [:unread]).uniq]
+    [returned_offset, (@labels + [:unread])]
   end
 end
 
diff --git a/lib/sup/message.rb b/lib/sup/message.rb
index 7c63746..3b10744 100644
--- a/lib/sup/message.rb
+++ b/lib/sup/message.rb
@@ -46,7 +46,7 @@ class Message
     @snippet = opts[:snippet]
     @snippet_contains_encrypted_content = false
     @have_snippet = !(opts[:snippet].nil? || opts[:snippet].empty?)
-    @labels = (opts[:labels] || []).to_set_of_symbols
+    @labels = Set.new(opts[:labels] || [])
     @dirty = false
     @encrypted = false
     @chunks = nil
@@ -165,14 +165,14 @@ class Message
   end
 
   def has_label? t; @labels.member? t; end
-  def add_label t
-    return if @labels.member? t
-    @labels = (@labels + [t]).to_set_of_symbols
+  def add_label l
+    return if @labels.member? l
+    @labels << l
     @dirty = true
   end
-  def remove_label t
-    return unless @labels.member? t
-    @labels.delete t
+  def remove_label l
+    return unless @labels.member? l
+    @labels.delete l
     @dirty = true
   end
 
@@ -181,7 +181,9 @@ class Message
   end
 
   def labels= l
-    @labels = l.to_set_of_symbols
+    raise ArgumentError, "not a set" unless l.is_a?(Set)
+    return if @labels == l
+    @labels = l
     @dirty = true
   end
 
diff --git a/lib/sup/modes/inbox-mode.rb b/lib/sup/modes/inbox-mode.rb
index d8daeb9..ba095da 100644
--- a/lib/sup/modes/inbox-mode.rb
+++ b/lib/sup/modes/inbox-mode.rb
@@ -1,4 +1,4 @@
-require 'thread'
+require 'sup'
 
 module Redwood
 
@@ -15,9 +15,7 @@ class InboxMode < ThreadIndexMode
     @@instance = self
   end
 
-  def is_relevant? m
-    m.has_label?(:inbox) && ([:spam, :deleted, :killed] & m.labels).empty?
-  end
+  def is_relevant? m; (m.labels & [:spam, :deleted, :killed, :inbox]) == Set.new([:inbox]) end
 
   ## label-list-mode wants to be able to raise us if the user selects
   ## the "inbox" label, so we need to keep our singletonness around
diff --git a/lib/sup/modes/thread-index-mode.rb b/lib/sup/modes/thread-index-mode.rb
index b671119..905ad98 100644
--- a/lib/sup/modes/thread-index-mode.rb
+++ b/lib/sup/modes/thread-index-mode.rb
@@ -533,9 +533,9 @@ EOS
     keepl, modifyl = thread.labels.partition { |t| speciall.member? t }
 
     user_labels = BufferManager.ask_for_labels :label, "Labels for thread: ", modifyl, @hidden_labels
-
     return unless user_labels
-    thread.labels = keepl + user_labels
+
+    thread.labels = Set.new(keepl) + user_labels
     user_labels.each { |l| LabelManager << l }
     update_text_for_line curpos
 
diff --git a/lib/sup/poll.rb b/lib/sup/poll.rb
index 8a9d218..0c46d2f 100644
--- a/lib/sup/poll.rb
+++ b/lib/sup/poll.rb
@@ -97,14 +97,14 @@ EOS
         numi = 0
         add_messages_from source do |m_old, m, offset|
           ## always preserve the labels on disk.
-          m.labels = ((m.labels - [:unread, :inbox]) + m_old.labels).uniq if m_old
-          yield "Found message at #{offset} with labels {#{m.labels * ', '}}"
+          m.labels = (m.labels - [:unread, :inbox]) + m_old.labels if m_old
+          yield "Found message at #{offset} with labels {#{m.labels.to_a * ', '}}"
           unless m_old
             num += 1
             from_and_subj << [m.from && m.from.longname, m.subj]
-            if m.has_label?(:inbox) && ([:spam, :deleted, :killed] & m.labels).empty?
+            if (m.labels & [:inbox, :spam, :deleted, :killed]) == Set.new([:inbox])
               from_and_subj_inbox << [m.from && m.from.longname, m.subj]
-              numi += 1 
+              numi += 1
             end
           end
           m
diff --git a/lib/sup/thread.rb b/lib/sup/thread.rb
index d395c35..1474b6e 100644
--- a/lib/sup/thread.rb
+++ b/lib/sup/thread.rb
@@ -24,6 +24,8 @@
 ## a faked root object tying them all together into one tree
 ## structure.
 
+require 'set'
+
 module Redwood
 
 class Thread
@@ -123,11 +125,10 @@ class Thread
 
   def size; map { |m, *o| m ? 1 : 0 }.sum; end
   def subj; argfind { |m, *o| m && m.subj }; end
-  def labels
-      map { |m, *o| m && m.labels }.flatten.compact.uniq.sort_by { |t| t.to_s }
-  end
+  def labels; inject(Set.new) { |s, (m, *o)| m ? s | m.labels : s } end
   def labels= l
-    each { |m, *o| m && m.labels = l.clone }
+    raise ArgumentError, "not a set" unless l.is_a?(Set)
+    each { |m, *o| m && m.labels = l.dup }
   end
 
   def latest_message
diff --git a/lib/sup/util.rb b/lib/sup/util.rb
index 3f2c901..c3cc4b2 100644
--- a/lib/sup/util.rb
+++ b/lib/sup/util.rb
@@ -2,6 +2,7 @@ require 'thread'
 require 'lockfile'
 require 'mime/types'
 require 'pathname'
+require 'set'
 
 ## time for some monkeypatching!
 class Lockfile
@@ -288,10 +289,12 @@ class String
     end
   end
 
-  ## takes a space-separated list of words, and returns an array of symbols.
-  ## typically used in Sup for translating Ferret's representation of a list
-  ## of labels (a string) to an array of label symbols.
-  def symbolistize; split.map { |x| x.intern } end
+  ## takes a list of words, and returns an array of symbols.  typically used in
+  ## Sup for translating Ferret's representation of a list of labels (a string)
+  ## to an array of label symbols.
+  ##
+  ## split_on will be passed to String#split, so you can leave this nil for space.
+  def to_set_of_symbols split_on=nil; Set.new split(split_on).map { |x| x.strip.intern } end
 end
 
 class Numeric
@@ -419,10 +422,6 @@ class Array
 
   def last= e; self[-1] = e end
   def nonempty?; !empty? end
-
-  def to_set_of_symbols
-    map { |x| x.is_a?(Symbol) ? x : x.intern }.uniq
-  end
 end
 
 class Time
diff --git a/lib/sup/xapian_index.rb b/lib/sup/xapian_index.rb
index 861c2a3..33f2204 100644
--- a/lib/sup/xapian_index.rb
+++ b/lib/sup/xapian_index.rb
@@ -100,7 +100,7 @@ class XapianIndex < BaseIndex
       :source_info => m.source_info,
       :date => (entry[:date] || m.date),
       :snippet => snippet,
-      :labels => labels.uniq,
+      :labels => labels,
       :from => (entry[:from] || [m.from.email, m.from.name]),
       :to => (entry[:to] || m.to.map { |p| [p.email, p.name] }),
       :cc => (entry[:cc] || m.cc.map { |p| [p.email, p.name] }),
@@ -110,7 +110,7 @@ class XapianIndex < BaseIndex
       :replytos => (entry[:replytos] || m.replytos),
     }
 
-    m.labels.each { |l| LabelManager << l }
+    labels.each { |l| LabelManager << l }
 
     synchronize do
       index_message m, opts