[sup-devel] [PATCHES] reply-mode, signatures and account selector

* [sup-devel] [PATCHES] reply-mode, signatures and account selector
@ 2010-06-09 12:53 Damien Leone
  2010-06-09 13:15 ` Damien Leone
  0 siblings, 1 reply; 3+ messages in thread
From: Damien Leone @ 2010-06-09 12:53 UTC (permalink / raw)
  To: sup-devel

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

Sup guys,

Three patches are attached to this email, the commit messages should
be self explanatory but here are some more details:

As the first patch changes the way headers are handled in reply-mode
there is a regression in the before-edit hook possibilities.
Before, we could write something like:

if header["Cc"] == "foo"
   header["Bcc"] = "bar"
end

Hence, switching the reply-to type using the selector would have
changed the Bcc field if a type using Cc was selected.

This would be no longer possible since there is now only one "set" of
headers running the hook, however if you properly wrote your reply-to
hook you can select the reply type you want and the previous code
will be working as expected.

On the other hand (in my opinion), the reply-mode now handles better
its job, that is to say changing To and Cc without interfering with
other fields that might have been edited manually by the user.

I asked rlane about this on IRC:
> <dleone> what is the reason for the before-edit hook being runned on @bodies and @header for each Reply-To types?
> <rlane> that's so that when you switch reply-to type the right signature/etc is displayed

I see no regression regarding this in the patch, so it should be okay.

Other patches add an account selector in edit-mode (it is useful to me
and I saw requests for such a feature) and a better signature handling
regarding the edit_signature option.

Regards,

[-- Attachment #2: 0001-reply-mode-improve-the-way-headers-are-handled.patch --]
[-- Type: application/octet-stream, Size: 5335 bytes --]

From d1fca8e404aee924dca432fc5f256ec363c61648 Mon Sep 17 00:00:00 2001
From: Damien Leone <damien.leone@fensalir.fr>
Date: Wed, 9 Jun 2010 12:38:52 +0200
Subject: [PATCH 1/3] reply-mode: improve the way headers are handled

I noticed that changing the Reply-to selector was also changing the
other headers such as From even if you edited it. I guess it is an
unwanted behaviour since this selector should only behave on the To
and Cc fields.

This commit splits the headers in two "types", those handled by
Reply-To selector and those that are only merged with the first ones
at initialization of the edit-mode.

It also gives a better support of the Customized choice by saving back
the fields for later use.
---
 lib/sup/modes/reply-mode.rb |   60 ++++++++++++++++++++++--------------------
 1 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/lib/sup/modes/reply-mode.rb b/lib/sup/modes/reply-mode.rb
index bbac922..8deef4e 100644
--- a/lib/sup/modes/reply-mode.rb
+++ b/lib/sup/modes/reply-mode.rb
@@ -48,6 +48,7 @@ EOS
     ## the full headers (most importantly the list-post header, if
     ## any)
     body = reply_body_lines message
+    @body_orig = body
 
     ## first, determine the address at which we received this email. this will
     ## become our From: address in the reply.
@@ -100,14 +101,21 @@ EOS
     @headers = {}
     @headers[:recipient] = {
       "To" => cc.map { |p| p.full_address },
+      "Cc" => [],
     } if useful_recipient
 
     ## typically we don't want to have a reply-to-sender option if the sender
     ## is a user account. however, if the cc is empty, it's a message to
     ## ourselves, so for the lack of any other options, we'll add it.
-    @headers[:sender] = { "To" => [to.full_address], } if !AccountManager.is_account?(to) || !useful_recipient
+    @headers[:sender] = {
+      "To" => [to.full_address],
+      "Cc" => [],
+    } if !AccountManager.is_account?(to) || !useful_recipient
 
-    @headers[:user] = {}
+    @headers[:user] = {
+      "To" => [],
+      "Cc" => [],
+    }
 
     not_me_ccs = cc.select { |p| !AccountManager.is_account?(p) }
     @headers[:all] = {
@@ -117,22 +125,11 @@ EOS
 
     @headers[:list] = {
       "To" => [@m.list_address.full_address],
+      "Cc" => [],
     } if @m.is_list_message?
 
     refs = gen_references
 
-    @headers.each do |k, v|
-      @headers[k] = {
-               "From" => from.full_address,
-               "To" => [],
-               "Cc" => [],
-               "Bcc" => [],
-               "In-reply-to" => "<#{@m.id}>",
-               "Subject" => Message.reify_subj(@m.subj),
-               "References" => refs,
-             }.merge v
-    end
-
     types = REPLY_TYPES.select { |t| @headers.member?(t) }
     @type_selector = HorizontalSelector.new "Reply to:", types, types.map { |x| TYPE_DESCRIPTIONS[x] }
 
@@ -151,13 +148,17 @@ EOS
         :recipient
       end)
 
-    @bodies = {}
-    @headers.each do |k, v|
-      @bodies[k] = body
-      HookManager.run "before-edit", :header => v, :body => @bodies[k]
-    end
+    headers_full = {
+      "From" => from.full_address,
+      "Bcc" => [],
+      "In-reply-to" => "<#{@m.id}>",
+      "Subject" => Message.reify_subj(@m.subj),
+      "References" => refs,
+    }.merge @headers[@type_selector.val]
+
+    HookManager.run "before-edit", :header => headers_full, :body => body
 
-    super :header => @headers[@type_selector.val], :body => @bodies[@type_selector.val], :twiddles => false
+    super :header => headers_full, :body => body, :twiddles => false
     add_selector @type_selector
   end
 
@@ -166,8 +167,7 @@ protected
   def move_cursor_right
     super
     if @headers[@type_selector.val] != self.header
-      self.header = @headers[@type_selector.val]
-      self.body = @bodies[@type_selector.val] unless @edited
+      self.header = self.header.merge @headers[@type_selector.val]
       update
     end
   end
@@ -175,8 +175,7 @@ protected
   def move_cursor_left
     super
     if @headers[@type_selector.val] != self.header
-      self.header = @headers[@type_selector.val]
-      self.body = @bodies[@type_selector.val] unless @edited
+      self.header = self.header.merge @headers[@type_selector.val]
       update
     end
   end
@@ -193,14 +192,15 @@ protected
   end
 
   def handle_new_text new_header, new_body
-    if new_body != @bodies[@type_selector.val]
-      @bodies[@type_selector.val] = new_body
+    if new_body != @body_orig
+      @body_orig = new_body
       @edited = true
     end
     old_header = @headers[@type_selector.val]
-    if new_header.size != old_header.size || old_header.any? { |k, v| new_header[k] != v }
+    if old_header.any? { |k, v| new_header[k] != v }
       @type_selector.set_to :user
-      self.header = @headers[:user] = new_header
+      self.header["To"] = @headers[:user]["To"] = new_header["To"]
+      self.header["Cc"] = @headers[:user]["Cc"] = new_header["Cc"]
       update
     end
   end
@@ -211,8 +211,10 @@ protected
 
   def edit_field field
     edited_field = super
-    if edited_field && edited_field != "Subject"
+    if edited_field and (field == "To" or field == "Cc")
       @type_selector.set_to :user
+      @headers[:user]["To"] = self.header["To"]
+      @headers[:user]["Cc"] = self.header["Cc"]
       update
     end
   end
-- 
1.7.1


[-- Attachment #3: 0002-Add-account_selector-config-option.patch --]
[-- Type: application/octet-stream, Size: 3643 bytes --]

From 4ceeba16d7030dab3567b502adc1f01b4807300d Mon Sep 17 00:00:00 2001
From: Damien Leone <damien.leone@fensalir.fr>
Date: Wed, 9 Jun 2010 12:51:20 +0200
Subject: [PATCH 2/3] Add account_selector config option

Set to true by default, it allows you to change your account in
edit-mode using a horizontal selector. It doesn't change any of the
previous behaviour since the proper account is selected by default, it
also handles customized choice in case the user edited the From field
manually.
---
 lib/sup.rb                         |    3 +-
 lib/sup/modes/edit-message-mode.rb |   37 ++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/lib/sup.rb b/lib/sup.rb
index c68251e..587760d 100644
--- a/lib/sup.rb
+++ b/lib/sup.rb
@@ -267,7 +267,8 @@ else
     :sent_source => "sup://sent",
     :poll_interval => 300,
     :wrap_width => 0,
-    :slip_rows => 0
+    :slip_rows => 0,
+    :account_selector => true
   }
   begin
     FileUtils.mkdir_p Redwood::BASE_DIR
diff --git a/lib/sup/modes/edit-message-mode.rb b/lib/sup/modes/edit-message-mode.rb
index bba07b5..f9e37b0 100644
--- a/lib/sup/modes/edit-message-mode.rb
+++ b/lib/sup/modes/edit-message-mode.rb
@@ -103,6 +103,21 @@ EOS
     @selectors = []
     @selector_label_width = 0
 
+    if $config[:account_selector]
+      @account_selector =
+        HorizontalSelector.new "Account:", AccountManager.user_accounts + [nil], AccountManager.user_emails + ["Customized"]
+
+      if @header["From"] =~ /<?(\S+@(\S+?))>?$/
+        @account_selector.set_to AccountManager.account_for($1)
+        @account_user = ""
+      else
+        @account_selector.set_to nil
+        @account_user = @header["From"]
+      end
+
+      add_selector @account_selector
+    end
+
     @crypto_selector =
       if CryptoManager.have_crypto?
         HorizontalSelector.new "Crypto:", [:none] + CryptoManager::OUTGOING_MESSAGE_OPERATIONS.keys, ["None"] + CryptoManager::OUTGOING_MESSAGE_OPERATIONS.values
@@ -151,6 +167,8 @@ EOS
   def edit_subject; edit_field "Subject" end
 
   def edit_message
+    old_from = @header["From"] if @account_selector
+
     @file = Tempfile.new "sup.#{self.class.name.gsub(/.*::/, '').camel_to_hyphy}"
     @file.puts format_headers(@header - NON_EDITABLE_HEADERS).first
     @file.puts
@@ -167,6 +185,12 @@ EOS
 
     header, @body = parse_file @file.path
     @header = header - NON_EDITABLE_HEADERS
+
+    if @account_selector and @header["From"] != old_from
+      @account_user = @header["From"]
+      @account_selector.set_to nil
+    end
+
     handle_new_text @header, @body
     update
 
@@ -231,6 +255,7 @@ protected
     if curpos < @selectors.length
       @selectors[curpos].roll_left
       buffer.mark_dirty
+      update if @account_selector
     else
       col_left
     end
@@ -240,6 +265,7 @@ protected
     if curpos < @selectors.length
       @selectors[curpos].roll_right
       buffer.mark_dirty
+      update if @account_selector
     else
       col_right
     end
@@ -251,6 +277,11 @@ protected
   end
 
   def update
+    if @account_selector
+      account = @account_selector.val
+      @header["From"] = account && account.full_address || @account_user
+    end
+
     regen_text
     buffer.mark_dirty if buffer
   end
@@ -455,6 +486,12 @@ protected
       if contacts
         text = contacts.map { |s| s.full_address }.join(", ")
         @header[field] = parse_header field, text
+
+        if @account_selector and field == "From"
+          @account_user = @header["From"]
+          @account_selector.set_to nil
+        end
+
         update
       end
     end
-- 
1.7.1


[-- Attachment #4: 0003-edit-mode-change-the-way-signatures-are-handled.patch --]
[-- Type: application/octet-stream, Size: 2855 bytes --]

From 40be99af5add5ff149933558c75c6aa63a9b2205 Mon Sep 17 00:00:00 2001
From: Damien Leone <damien.leone@fensalir.fr>
Date: Wed, 9 Jun 2010 12:57:18 +0200
Subject: [PATCH 3/3] edit-mode: change the way signatures are handled

The current signatures handling was not suitable for account changing
in edit-mode. It was working fine when the edit_signature option was
false, but it could have a better behaviour if this option was
enabled.

This commit tries to do this by appending the signature to the body
text if the edit_signature is true, then after editing the message it
checks if the signature has been modified by comparing the end of the
file to the current account's signature. If it has been edited then we
stick to it by setting @sig_edited to true, otherwise the signature
will still be automatically changed if another account is selected.
---
 lib/sup/modes/edit-message-mode.rb |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/sup/modes/edit-message-mode.rb b/lib/sup/modes/edit-message-mode.rb
index f9e37b0..00d6ba9 100644
--- a/lib/sup/modes/edit-message-mode.rb
+++ b/lib/sup/modes/edit-message-mode.rb
@@ -81,7 +81,7 @@ EOS
     @header_lines = []
 
     @body = opts.delete(:body) || []
-    @body += sig_lines if $config[:edit_signature] && !opts.delete(:have_signature)
+    @sig_edited = false
 
     if opts[:attachments]
       @attachments = opts[:attachments].values
@@ -167,12 +167,14 @@ EOS
   def edit_subject; edit_field "Subject" end
 
   def edit_message
+    sig = sig_lines.join("\n")
     old_from = @header["From"] if @account_selector
 
     @file = Tempfile.new "sup.#{self.class.name.gsub(/.*::/, '').camel_to_hyphy}"
     @file.puts format_headers(@header - NON_EDITABLE_HEADERS).first
     @file.puts
     @file.puts @body.join("\n")
+    @file.puts sig if ($config[:edit_signature] and !@sig_edited)
     @file.close
 
     editor = $config[:editor] || ENV['EDITOR'] || "/usr/bin/vi"
@@ -186,6 +188,19 @@ EOS
     header, @body = parse_file @file.path
     @header = header - NON_EDITABLE_HEADERS
 
+    if $config[:edit_signature]
+      pbody = @body.join("\n")
+      blen = pbody.length
+      slen = sig.length
+
+      if blen > slen and pbody[blen-slen..blen] == sig
+        @sig_edited = false
+        @body = pbody[0..blen-slen].split("\n")
+      else
+        @sig_edited = true
+      end
+    end
+
     if @account_selector and @header["From"] != old_from
       @account_user = @header["From"]
       @account_selector.set_to nil
@@ -289,7 +304,7 @@ protected
   def regen_text
     header, @header_lines = format_headers(@header - NON_EDITABLE_HEADERS) + [""]
     @text = header + [""] + @body
-    @text += sig_lines unless $config[:edit_signature]
+    @text += sig_lines unless @sig_edited
     
     @attachment_lines_offset = 0
 
-- 
1.7.1


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

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

^ permalink raw reply	[flat|nested] 3+ messages in thread