Subject: [sup-talk] [PATCH] Fix a bug about forwarding that trimmed out newlines.

From: [email-masked] (Nicolas Pouillard)
Date: Fri, 4 Apr 2008 18:09:13 +0200

This seems to happen when one don't touch the forwarded
email. Indeed most of the sup's code tend to assume that
messages body are reprensented using an array of string
*without* a trailing \n, except some parts of the code.

This patch enforce that. It could become even more strict
by raising an error instead of logging in join_lines.
---
 lib/sup/modes/edit-message-mode.rb |   11 +++++------
 lib/sup/util.rb                    |    6 ++++++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/lib/sup/modes/edit-message-mode.rb b/lib/sup/modes/edit-message-mode.rb
index dd96002..c3634af 100644
--- a/lib/sup/modes/edit-message-mode.rb
+++ b/lib/sup/modes/edit-message-mode.rb
@@ -216,7 +216,7 @@ protected
   def parse_file fn
     File.open(fn) do |f|
       header = MBox::read_header f
-      body = f.readlines
+      body = f.readlines.map { |l| l.chomp }
 
       header.delete_if { |k, v| NON_EDITABLE_HEADERS.member? k }
       header.each { |k, v| header[k] = parse_header k, v }
@@ -307,9 +307,8 @@ protected
   def build_message date
     m = RMail::Message.new
     m.header["Content-Type"] = "text/plain; charset=#{$encoding}"
-    m.body = @body.join
-    m.body = m.body
-    m.body += sig_lines.join("\n") unless $config[:edit_signature]
+    m.body = @body.join_lines
+    m.body += sig_lines.join_lines unless $config[:edit_signature]
 
     ## there are attachments, so wrap body in an attachment of its own
     unless @attachments.empty?
@@ -367,7 +366,7 @@ EOS
     end
 
     f.puts
-    f.puts sanitize_body(@body.join)
+    f.puts sanitize_body(@body.join_lines)
     f.puts sig_lines if full unless $config[:edit_signature]
   end  
 
@@ -412,7 +411,7 @@ private
   end
 
   def top_posting?
-    @body.join =~ /(\S+)\s*Excerpts from.*\n(>.*\n)+\s*\Z/
+    @body.join_lines =~ /(\S+)\s*Excerpts from.*\n(>.*\n)+\s*\Z/
   end
 
   def sig_lines
diff --git a/lib/sup/util.rb b/lib/sup/util.rb
index 99e73b4..544859c 100644
--- a/lib/sup/util.rb
+++ b/lib/sup/util.rb
@@ -390,6 +390,12 @@ module Enumerable
 end
 
 class Array
+
+  def join_lines
+    Redwood::log "Some newlines are there..." if any? { |l| l =~ /\n/ }
+    map { |l| l.chomp }.join("\n")
+  end
+
   def flatten_one_level
     inject([]) { |a, e| a + e }
   end
-- 
1.5.5.rc3

---

From: [email-masked] (William Morgan)
Date: Fri, 04 Apr 2008 10:47:31 -0700

Reformatted excerpts from nicolas.pouillard's message of 2008-04-04:
> This seems to happen when one don't touch the forwarded email. Indeed
> most of the sup's code tend to assume that messages body are
> reprensented using an array of string *without* a trailing \n, except
> some parts of the code.

I see. I think this patch is the right approach.

> This patch enforce that. It could become even more strict by raising
> an error instead of logging in join_lines.

Does that log message ever get triggered for you? I'd prefer to have an
exception thrown, but only if it will never happen. :)

-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Nicolas Pouillard)
Date: Sat, 05 Apr 2008 11:26:19 +0200

> exception thrown, but only if it will never happen. :)

I  will  change  it  to  an  exception for some time, and if it's good, I will
submit a patch. For now I think this patch is the safe way.

-- 
Nicolas Pouillard aka Ertai
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 286 bytes
Desc: not available
Url : http://rubyforge.org/pipermail/sup-talk/attachments/20080405/6d0c0693/attachment.bin

---

From: [email-masked] (Nicolas Pouillard)
Date: Mon, 21 Apr 2008 10:02:46 +0200

What about this patch?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: not available
Url : http://rubyforge.org/pipermail/sup-talk/attachments/20080421/85e19dca/attachment-0001.bin

---

From: [email-masked] (William Morgan)
Date: Tue, 22 Apr 2008 18:44:26 -0700

> What about this patch?

I merged a more minor version of the fix into next.

---

From: [email-masked] (Nicolas Pouillard)
Date: Wed, 23 Apr 2008 10:59:24 +0200

> I merged a more minor version of the fix into next.

Great!

However this hunk is useless:

====================================================================================
--- a/lib/sup/modes/edit-message-mode.rb
+++ b/lib/sup/modes/edit-message-mode.rb
@@ -122,7 +122,7 @@ EOS
     @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
+    @file.puts @body.join("\n")
     @file.close
 
     editor = $config[:editor] || ENV['EDITOR'] || "/usr/bin/vi"
====================================================================================

Since  puts of an array have the same semantics, but cheaper. However it worth
a comment to make it crystal clear.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: not available
Url : http://rubyforge.org/pipermail/sup-talk/attachments/20080423/d3ac93fb/attachment.bin