clean up undo logic

Various changes:
- Use UndoManager#register block form where possible
- Clean up pluralization code
- Clean up undo return values
- Minor logic fixes

Still some work to do here to make undo lambda returning
more consistent.

diff --git a/lib/sup/modes/inbox-mode.rb b/lib/sup/modes/inbox-mode.rb
index 21eb9ac43bff5cbd01fb0e10807b61381aabf1bd..d8daeb9c4cbe19ad1fc2644aa1d5c84a4e382f23 100644 (file)
--- a/lib/sup/modes/inbox-mode.rb
+++ b/lib/sup/modes/inbox-mode.rb
@@ -28,11 +28,10 @@ class InboxMode < ThreadIndexMode
     return unless cursor_thread
     thread = cursor_thread # to make sure lambda only knows about 'old' cursor_thread
 
-    undo = lambda {
+    UndoManager.register "archiving thread" do
       thread.apply_label :inbox
       add_or_unhide thread.first
-    }
-    UndoManager.register("archiving thread #{thread.first.id}", undo)
+    end
 
     cursor_thread.remove_label :inbox
     hide_thread cursor_thread
@@ -40,13 +39,13 @@ class InboxMode < ThreadIndexMode
   end
 
   def multi_archive threads
-    undo = threads.map {|t|
-             lambda{
-               t.apply_label :inbox
-               add_or_unhide t.first
-             }}
-    UndoManager.register("archiving #{threads.size} #{threads.size.pluralize 'thread'}",
-                         undo << lambda {regen_text} )
+    UndoManager.register "archiving #{threads.size.pluralize 'thread'}" do
+      threads.map do |t|
+        t.apply_label :inbox
+        add_or_unhide t.first
+      end
+      regen_text
+    end
 
     threads.each do |t|
       t.remove_label :inbox
@@ -59,12 +58,11 @@ class InboxMode < ThreadIndexMode
     return unless cursor_thread
     thread = cursor_thread # to make sure lambda only knows about 'old' cursor_thread
 
-    undo = lambda {
+    UndoManager.register "reading and archiving thread" do
       thread.apply_label :inbox
       thread.apply_label :unread
       add_or_unhide thread.first
-    }
-    UndoManager.register("reading and archiving thread ", undo)
+    end
 
     cursor_thread.remove_label :unread
     cursor_thread.remove_label :inbox
@@ -73,15 +71,7 @@ class InboxMode < ThreadIndexMode
   end
 
   def multi_read_and_archive threads
-    undo = threads.map {|t|
-      lambda {
-        t.apply_label :inbox
-        t.apply_label :unread
-        add_or_unhide t.first
-      }
-    }
-    UndoManager.register("reading and archiving #{threads.size} #{threads.size.pluralize 'thread'}",
-                         undo << lambda {regen_text})
+    old_labels = threads.map { |t| t.labels.dup }
 
     threads.each do |t|
       t.remove_label :unread
@@ -89,6 +79,15 @@ class InboxMode < ThreadIndexMode
       hide_thread t
     end
     regen_text
+
+    UndoManager.register "reading and archiving #{threads.size.pluralize 'thread'}" do
+      threads.zip(old_labels).each do |t, l|
+        t.labels = l
+        add_or_unhide t.first
+      end
+      regen_text
+    end
+
   end
 
   def handle_unarchived_update sender, m

diff --git a/lib/sup/modes/thread-index-mode.rb b/lib/sup/modes/thread-index-mode.rb
index f28302bb2e49b85ec3fbb735de63c6f14c0ced43..6152666d2e8398e1e76a1d9085b7ac7838965993 100644 (file)
--- a/lib/sup/modes/thread-index-mode.rb
+++ b/lib/sup/modes/thread-index-mode.rb
@@ -236,129 +236,122 @@ EOS
     end
   end
 
+  ## returns an undo lambda
   def actually_toggle_starred t
-    thread = t # cargo cult programming
     pos = curpos
     if t.has_label? :starred # if ANY message has a star
-      undo = lambda {
-        thread.first.add_label :starred
-        update_text_for_line pos
-        UpdateManager.relay self, :starred, thread.first
-      }
       t.remove_label :starred # remove from all
       UpdateManager.relay self, :unstarred, t.first
+      lambda do
+        t.first.add_label :starred
+        UpdateManager.relay self, :starred, t.first
+        regen_text
+      end
     else
-      undo = lambda {
-        thread.remove_label :starred
-        update_text_for_line pos
-        UpdateManager.relay self, :unstarred, thread.first
-      }
       t.first.add_label :starred # add only to first
       UpdateManager.relay self, :starred, t.first
+      lambda do
+        t.remove_label :starred
+        UpdateManager.relay self, :unstarred, t.first
+        regen_text
+      end
     end
-
-    return undo
   end  
 
   def toggle_starred 
     t = cursor_thread or return
     undo = actually_toggle_starred t
-    UndoManager.register("starring/unstarring thread #{t.first.id}",undo)
+    UndoManager.register "toggling thread starred status", undo
     update_text_for_line curpos
     cursor_down
   end
 
   def multi_toggle_starred threads
-    undo = threads.map { |t| actually_toggle_starred t }
-    UndoManager.register("starring/unstarring #{threads.size} #{threads.size.pluralize 'thread'}",
-                         undo)
+    UndoManager.register "toggling #{threads.size.pluralize 'thread'} starred status",
+      threads.map { |t| actually_toggle_starred t }
     regen_text
   end
 
+  ## returns an undo lambda
   def actually_toggle_archived t
     thread = t
     pos = curpos
     if t.has_label? :inbox
       t.remove_label :inbox
-      undo = lambda {
+      UpdateManager.relay self, :archived, t.first
+      lambda do
         thread.apply_label :inbox
         update_text_for_line pos
         UpdateManager.relay self,:unarchived, thread.first
-      }
-      UpdateManager.relay self, :archived, t.first
+      end
     else
       t.apply_label :inbox
-      undo = lambda {
+      UpdateManager.relay self, :unarchived, t.first
+      lambda do
         thread.remove_label :inbox
         update_text_for_line pos
         UpdateManager.relay self, :unarchived, thread.first
-      }
-      UpdateManager.relay self, :unarchived, t.first
+      end
     end
-
-    return undo
   end
 
+  ## returns an undo lambda
   def actually_toggle_spammed t
     thread = t
     if t.has_label? :spam
-      undo = lambda {
-        thread.apply_label :spam
-        self.hide_thread thread
-        UpdateManager.relay self,:spammed, thread.first
-      }
       t.remove_label :spam
       add_or_unhide t.first
       UpdateManager.relay self, :unspammed, t.first
+      lambda do
+        thread.apply_label :spam
+        self.hide_thread thread
+        UpdateManager.relay self,:spammed, thread.first
+      end
     else
-      undo = lambda {
-        thread.remove_label :spam
-        add_or_unhide thread.first
-        UpdateManager.relay self,:unspammed, thread.first
-      }
       t.apply_label :spam
       hide_thread t
       UpdateManager.relay self, :spammed, t.first
+      lambda do
+        thread.remove_label :spam
+        add_or_unhide thread.first
+        UpdateManager.relay self,:unspammed, thread.first
+      end
     end
-
-    return undo
   end
 
+  ## returns an undo lambda
   def actually_toggle_deleted t
     if t.has_label? :deleted
-      undo = lambda {
-        t.apply_label :deleted
-        hide_thread t
-        UpdateManager.relay self, :deleted, t.first
-      }
       t.remove_label :deleted
       add_or_unhide t.first
       UpdateManager.relay self, :undeleted, t.first
+      lambda do
+        t.apply_label :deleted
+        hide_thread t
+        UpdateManager.relay self, :deleted, t.first
+      end
     else
-      undo = lambda {
+      t.apply_label :deleted
+      hide_thread t
+      UpdateManager.relay self, :deleted, t.first
+      lambda do
         t.remove_label :deleted
         add_or_unhide t.first
         UpdateManager.relay self, :undeleted, t.first
-      }
-      t.apply_label :deleted
-  hide_thread t
-      UpdateManager.relay self, :deleted, t.first
+      end
     end
-
-    return undo
   end
 
   def toggle_archived 
     t = cursor_thread or return
-    undo = [actually_toggle_archived(t), lambda {self.update_text_for_line curpos}]
-    UndoManager.register("deleting/undeleting thread #{t.first.id}",undo)
+    undo = actually_toggle_archived t
+    UndoManager.register "deleting/undeleting thread #{t.first.id}", undo, lambda { update_text_for_line curpos }
     update_text_for_line curpos
   end
 
   def multi_toggle_archived threads
-    undo = threads.map { |t| actually_toggle_archived t}
-    UndoManager.register("deleting/undeleting #{threads.size} #{threads.size.pluralize 'thread'}",
-                         undo << lambda {self.regen_text})
+    undos = threads.map { |t| actually_toggle_archived t }
+    UndoManager.register "deleting/undeleting #{threads.size.pluralize 'thread'}", undos, lambda { regen_text }
     regen_text
   end
 
@@ -419,9 +412,9 @@ EOS
   ## see deleted or spam emails, and when you undelete or unspam them
   ## you also want them to disappear immediately.
   def multi_toggle_spam threads
-    undo = threads.map{ |t| actually_toggle_spammed t}
-    UndoManager.register("marking/unmarking #{threads.size} #{threads.size.pluralize 'thread'} as spam",
-                         undo <<  lambda {self.regen_text})
+    undos = threads.map { |t| actually_toggle_spammed t }
+    UndoManager.register "marking/unmarking  #{threads.size.pluralize 'thread'} as spam",
+                         undos, lambda { regen_text }
     regen_text
   end
 
@@ -432,9 +425,9 @@ EOS
 
   ## see comment for multi_toggle_spam
   def multi_toggle_deleted threads
-    undo = threads.map{ |t| actually_toggle_deleted t}
-    UndoManager.register("deleting/undeleting #{threads.size} #{threads.size.pluralize 'thread'}",
-                         undo << lambda {regen_text})
+    undos = threads.map { |t| actually_toggle_deleted t }
+    UndoManager.register "deleting/undeleting #{threads.size.pluralize 'thread'}",
+                         undos, lambda { regen_text }
     regen_text
   end
 
@@ -445,18 +438,21 @@ EOS
 
   ## m-m-m-m-MULTI-KILL
   def multi_kill threads
-    undo = threads.map do |t|
+    UndoManager.register "killing #{threads.size.pluralize 'thread'}" do
+      threads.each do |t|
+        t.remove_label :killed
+        add_or_unhide t.first
+      end
+      regen_text
+    end
+
+    threads.each do |t|
       t.apply_label :killed
       hide_thread t
-      thread = t
-      lambda { thread.remove_label :killed
-        add_or_unhide thread.first
-      }
     end
-    UndoManager.register("killing #{threads.size} #{threads.size.pluralize 'thread'}",
-                         undo << lambda {regen_text})
+
     regen_text
-    BufferManager.flash "#{threads.size.pluralize 'Thread'} killed."
+    BufferManager.flash "#{threads.size.pluralize 'thread'} killed."
   end
 
   def save
@@ -522,13 +518,11 @@ EOS
     user_labels.each { |l| LabelManager << l }
     update_text_for_line curpos
 
-    undo = lambda{
+    UndoManager.register "labeling thread #{thread.first.id}" do
       thread.labels = old_labels
       update_text_for_line pos
       UpdateManager.relay self, :labeled, thread.first
-    }
-
-    UndoManager.register("labeling thread #{thread.first.id}", undo)
+    end
 
     UpdateManager.relay self, :labeled, thread.first
   end
@@ -538,23 +532,30 @@ EOS
     return unless user_labels
     
     hl = user_labels.select { |l| @hidden_labels.member? l }
-    if hl.empty?
-      undo = threads.map { |t| old_labels = t.labels
-        user_labels.each { |l| t.apply_label l }
-        ## UpdateManager or some other regresh mechanism?
-        UpdateManager.relay self, :labeled, t.first
-        lambda {
-          t.labels = old_labels
-          UpdateManager.relay self, :labeled, t.first
-        }
-      }
-    user_labels.each { |l| LabelManager << l }
-    UndoManager.register("labeling #{threads.size} #{threads.size.pluralize 'thread'}",
-                         undo << lambda { regen_text})
-    else
+    unless hl.empty?
       BufferManager.flash "'#{hl}' is a reserved label!"
+      return
     end
+
+    old_labels = threads.map { |t| t.labels.dup }
+
+    threads.each do |t|
+      user_labels.each do |l|
+        t.apply_label l
+        LabelManager << l
+        UpdateManager.relay self, :labeled, t.first
+      end
+    end
+
     regen_text
+
+    UndoManager.register "labeling #{threads.size.pluralize 'thread'}" do
+      threads.zip(old_labels).map do |t, old_labels|
+        t.labels = old_labels
+        UpdateManager.relay self, :labeled, t.first
+      end
+      regen_text
+    end
   end
 
   def reply
@@ -758,7 +759,7 @@ protected
 
     date = t.date.to_nice_s
 
-    starred = t.has_label?(:starred)
+    starred = t.has_label? :starred
 
     ## format the from column
     cur_width = 0