Archive of RubyForge sup-talk mailing list
 help / color / mirror / Atom feed
* [sup-talk] [PATCH] Fix a bug about forwarding that trimmed out newlines.
@ 2008-04-04 16:16 kendall at clarkparsia.com
  0 siblings, 0 replies; 7+ messages in thread
From: kendall at clarkparsia.com @ 2008-04-04 16:16 UTC (permalink / raw)


On Friday, April 4, 2008 12:09pm, Nicolas Pouillard <nicolas.pouillard at gmail.com> said:

> 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 confirm this. It happens to me when I forward messages without editing them. It does not happen to messages that I edit (in any way).

Cheers,
Kendall



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

* [sup-talk] [PATCH] Fix a bug about forwarding that trimmed out newlines.
  2008-04-23  1:44   ` William Morgan
@ 2008-04-23  8:59     ` Nicolas Pouillard
  0 siblings, 0 replies; 7+ messages in thread
From: Nicolas Pouillard @ 2008-04-23  8:59 UTC (permalink / raw)


Excerpts from William Morgan's message of Wed Apr 23 03:44:26 +0200 2008:
> Reformatted excerpts from nicolas.pouillard's message of 2008-04-21:
> > What about this patch?
> 
> 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.

-- 
Nicolas Pouillard aka Ertai
-------------- 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 


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

* [sup-talk] [PATCH] Fix a bug about forwarding that trimmed out newlines.
  2008-04-21  8:02 ` Nicolas Pouillard
@ 2008-04-23  1:44   ` William Morgan
  2008-04-23  8:59     ` Nicolas Pouillard
  0 siblings, 1 reply; 7+ messages in thread
From: William Morgan @ 2008-04-23  1:44 UTC (permalink / raw)


Reformatted excerpts from nicolas.pouillard's message of 2008-04-21:
> What about this patch?

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

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


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

* [sup-talk] [PATCH] Fix a bug about forwarding that trimmed out newlines.
  2008-04-04 16:09 Nicolas Pouillard
  2008-04-04 17:47 ` William Morgan
@ 2008-04-21  8:02 ` Nicolas Pouillard
  2008-04-23  1:44   ` William Morgan
  1 sibling, 1 reply; 7+ messages in thread
From: Nicolas Pouillard @ 2008-04-21  8:02 UTC (permalink / raw)


What about this patch?

Excerpts from Nicolas Pouillard's message of Fri Apr 04 18:09:13 +0200 2008:
> 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

-- 
Nicolas Pouillard aka Ertai
-------------- 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 


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

* [sup-talk] [PATCH] Fix a bug about forwarding that trimmed out newlines.
  2008-04-04 17:47 ` William Morgan
@ 2008-04-05  9:26   ` Nicolas Pouillard
  0 siblings, 0 replies; 7+ messages in thread
From: Nicolas Pouillard @ 2008-04-05  9:26 UTC (permalink / raw)


Excerpts from William Morgan's message of Fri Apr 04 19:47:31 +0200 2008:
> 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. :)

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 


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

* [sup-talk] [PATCH] Fix a bug about forwarding that trimmed out newlines.
  2008-04-04 16:09 Nicolas Pouillard
@ 2008-04-04 17:47 ` William Morgan
  2008-04-05  9:26   ` Nicolas Pouillard
  2008-04-21  8:02 ` Nicolas Pouillard
  1 sibling, 1 reply; 7+ messages in thread
From: William Morgan @ 2008-04-04 17:47 UTC (permalink / raw)


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>


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

* [sup-talk] [PATCH] Fix a bug about forwarding that trimmed out newlines.
@ 2008-04-04 16:09 Nicolas Pouillard
  2008-04-04 17:47 ` William Morgan
  2008-04-21  8:02 ` Nicolas Pouillard
  0 siblings, 2 replies; 7+ messages in thread
From: Nicolas Pouillard @ 2008-04-04 16:09 UTC (permalink / raw)


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



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

end of thread, other threads:[~2008-04-23  8:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-04-04 16:16 [sup-talk] [PATCH] Fix a bug about forwarding that trimmed out newlines kendall at clarkparsia.com
  -- strict thread matches above, loose matches on Subject: below --
2008-04-04 16:09 Nicolas Pouillard
2008-04-04 17:47 ` William Morgan
2008-04-05  9:26   ` Nicolas Pouillard
2008-04-21  8:02 ` Nicolas Pouillard
2008-04-23  1:44   ` William Morgan
2008-04-23  8:59     ` Nicolas Pouillard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox