Archive of RubyForge sup-talk mailing list
 help / color / mirror / Atom feed
* [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset
@ 2009-07-10 15:00 Adeodato Simó
  2009-07-23  9:35 ` Adeodato Simó
  0 siblings, 1 reply; 11+ messages in thread
From: Adeodato Simó @ 2009-07-10 15:00 UTC (permalink / raw)


This is useful, for example, for HTML attachments which are sent in a
charset different from the default for the system (eg., ISO-8859-1 on an
UTF-8 system), so that the converter program can be told what charset it
should be converting from.

Signed-off-by: Adeodato Sim? <dato at net.com.org.es>
---
 lib/sup/message-chunks.rb |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/lib/sup/message-chunks.rb b/lib/sup/message-chunks.rb
index 0d742d9..ea04ed7 100644
--- a/lib/sup/message-chunks.rb
+++ b/lib/sup/message-chunks.rb
@@ -50,7 +50,8 @@ directly in Sup. For attachments that you wish to use a separate program
 to view (e.g. images), you should use the mime-view hook instead.
 
 Variables:
-   content_type: the content-type of the message
+   content_type: the content-type of the attachment
+        charset: the charset of the attachment, if applicable
        filename: the filename of the attachment as saved to disk
   sibling_types: if this attachment is part of a multipart MIME attachment,
                  an array of content-types for all attachments. Otherwise,
@@ -103,6 +104,7 @@ EOS
         else
           HookManager.run "mime-decode", :content_type => content_type,
                           :filename => lambda { write_to_disk },
+                          :charset => encoded_content.charset,
                           :sibling_types => sibling_types
         end
 
-- 
1.6.3.3



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

* [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset
  2009-07-10 15:00 [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset Adeodato Simó
@ 2009-07-23  9:35 ` Adeodato Simó
  2009-07-27 16:51   ` William Morgan
  0 siblings, 1 reply; 11+ messages in thread
From: Adeodato Simó @ 2009-07-23  9:35 UTC (permalink / raw)


+ Adeodato Sim? (Fri, 10 Jul 2009 17:00:29 +0200):

> +                          :charset => encoded_content.charset,

Hm, so apparently encoded_content doesn't always have a charset
member...

-- 
- Are you sure we're good?
- Always.
        -- Rory and Lorelai



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

* [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset
  2009-07-23  9:35 ` Adeodato Simó
@ 2009-07-27 16:51   ` William Morgan
  2009-07-30 15:56     ` Adeodato Simó
  0 siblings, 1 reply; 11+ messages in thread
From: William Morgan @ 2009-07-27 16:51 UTC (permalink / raw)


Reformatted excerpts from Adeodato Sim?'s message of 2009-07-23:
> + Adeodato Sim? (Fri, 10 Jul 2009 17:00:29 +0200):
> 
> > +                          :charset => encoded_content.charset,
> 
> Hm, so apparently encoded_content doesn't always have a charset
> member...
> 

In which case that value of that variable is nil, right? Is that a
problem? The patch still seems useful.
-- 
William <wmorgan-sup at masanjin.net>


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

* [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset
  2009-07-27 16:51   ` William Morgan
@ 2009-07-30 15:56     ` Adeodato Simó
  2009-07-30 15:57       ` [sup-talk] [PATCH] HookContext::method_missing(): allow locals to be nil Adeodato Simó
  2009-08-19 21:22       ` [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset Adeodato Simó
  0 siblings, 2 replies; 11+ messages in thread
From: Adeodato Simó @ 2009-07-30 15:56 UTC (permalink / raw)


+ William Morgan (Mon, 27 Jul 2009 09:51:17 -0700):

> Reformatted excerpts from Adeodato Sim?'s message of 2009-07-23:
> > + Adeodato Sim? (Fri, 10 Jul 2009 17:00:29 +0200):
> > > +                          :charset => encoded_content.charset,

> > Hm, so apparently encoded_content doesn't always have a charset
> > member...

> In which case that value of that variable is nil, right? Is that a
> problem? The patch still seems useful.

Yes, I took a closer look and you're right the result of
encoded_content.charset is nil in that case. I also saw (I think) where
the traceback I was seeing is coming from: apparently it's not possible
to pass to HookContext a local that is nil, since then "super" will get
called in HookContext::method_missing() as far as I can see.

So, perhaps, to fix this patch, one could do:

    :charset => encoded_content.charset || ''

But then, I think it would be better to fix HookContext to allow for
@__locals to contain nil. I'm not very familiar with that class, but it
seems easy enough to fix, see upcoming patch (also found in
~dato/sup/sup-dato:fixes at Gitorious, if you prefer that). With it,
this improvement to mime-decode seems to work without any further
trouble.

Cheers,

-- 
- Are you sure we're good?
- Always. -- Rory and Lorelai



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

* [sup-talk] [PATCH] HookContext::method_missing(): allow locals to be nil
  2009-07-30 15:56     ` Adeodato Simó
@ 2009-07-30 15:57       ` Adeodato Simó
  2009-08-19 21:22       ` [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset Adeodato Simó
  1 sibling, 0 replies; 11+ messages in thread
From: Adeodato Simó @ 2009-07-30 15:57 UTC (permalink / raw)


With the previous implementation of method_missing() in HookContext, it
was not possible to set the value of a local to nil, since that would be
assumed to mean "that local does not exist" and super would get called.

Now we use has_key? to check whether the local exists or not, and nil
will be returned normally from method_missing if that's the value of the
@__locals[name].

Signed-off-by: Adeodato Sim? <dato at net.com.org.es>
---
 lib/sup/hook.rb |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/sup/hook.rb b/lib/sup/hook.rb
index 0a0a2f6..8a52a96 100644
--- a/lib/sup/hook.rb
+++ b/lib/sup/hook.rb
@@ -20,13 +20,15 @@ class HookManager
     attr_writer :__locals
 
     def method_missing m, *a
-      case @__locals[m]
-      when Proc
-        @__locals[m] = @__locals[m].call(*a) # only call the proc once
-      when nil
+      if not @__locals.has_key? m
         super
       else
-        @__locals[m]
+        case @__locals[m]
+        when Proc
+          @__locals[m] = @__locals[m].call(*a) # only call the proc once
+        else
+          @__locals[m]
+        end
       end
     end
 
-- 
1.6.3.3



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

* [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset
  2009-07-30 15:56     ` Adeodato Simó
  2009-07-30 15:57       ` [sup-talk] [PATCH] HookContext::method_missing(): allow locals to be nil Adeodato Simó
@ 2009-08-19 21:22       ` Adeodato Simó
  2009-08-22 18:15         ` William Morgan
  1 sibling, 1 reply; 11+ messages in thread
From: Adeodato Simó @ 2009-08-19 21:22 UTC (permalink / raw)


Ping?

+ Adeodato Sim? (Thu, 30 Jul 2009 17:56:56 +0200):

> + William Morgan (Mon, 27 Jul 2009 09:51:17 -0700):

> > Reformatted excerpts from Adeodato Sim?'s message of 2009-07-23:
> > > + Adeodato Sim? (Fri, 10 Jul 2009 17:00:29 +0200):
> > > > +                          :charset => encoded_content.charset,

> > > Hm, so apparently encoded_content doesn't always have a charset
> > > member...

> > In which case that value of that variable is nil, right? Is that a
> > problem? The patch still seems useful.

> Yes, I took a closer look and you're right the result of
> encoded_content.charset is nil in that case. I also saw (I think) where
> the traceback I was seeing is coming from: apparently it's not possible
> to pass to HookContext a local that is nil, since then "super" will get
> called in HookContext::method_missing() as far as I can see.

> So, perhaps, to fix this patch, one could do:

>     :charset => encoded_content.charset || ''

> But then, I think it would be better to fix HookContext to allow for
> @__locals to contain nil. I'm not very familiar with that class, but it
> seems easy enough to fix, see upcoming patch (also found in
> ~dato/sup/sup-dato:fixes at Gitorious, if you prefer that). With it,
> this improvement to mime-decode seems to work without any further
> trouble.

> Cheers,


-- 
- Are you sure we're good?
- Always.
        -- Rory and Lorelai



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

* [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset
  2009-08-19 21:22       ` [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset Adeodato Simó
@ 2009-08-22 18:15         ` William Morgan
  2009-08-22 19:16           ` Adeodato Simó
  0 siblings, 1 reply; 11+ messages in thread
From: William Morgan @ 2009-08-22 18:15 UTC (permalink / raw)


Reformatted excerpts from Adeodato Sim?'s message of 2009-08-19:
> Ping?

Sorry, working on a fix for the hook variables issue. Bug me again in a
few days if I haven't gotten to this.
-- 
William <wmorgan-sup at masanjin.net>


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

* [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset
  2009-08-22 18:15         ` William Morgan
@ 2009-08-22 19:16           ` Adeodato Simó
  2009-08-23 17:34             ` William Morgan
  0 siblings, 1 reply; 11+ messages in thread
From: Adeodato Simó @ 2009-08-22 19:16 UTC (permalink / raw)


+ William Morgan (Sat, 22 Aug 2009 11:15:15 -0700):

> Reformatted excerpts from Adeodato Sim?'s message of 2009-08-19:
> > Ping?

> Sorry, working on a fix for the hook variables issue. Bug me again in a
> few days if I haven't gotten to this.

Only in case it needs saying: you mean a fix other than the patch I sent
for that as well? (Perhaps my proposed fix was not completed, I just
want to make sure the patch got noticed.)

Thanks,

-- 
- Are you sure we're good?
- Always.
        -- Rory and Lorelai



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

* [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset
  2009-08-22 19:16           ` Adeodato Simó
@ 2009-08-23 17:34             ` William Morgan
  2009-09-13 23:17               ` Adeodato Simó
  0 siblings, 1 reply; 11+ messages in thread
From: William Morgan @ 2009-08-23 17:34 UTC (permalink / raw)


Reformatted excerpts from Adeodato Sim?'s message of 2009-08-22:
> Only in case it needs saying: you mean a fix other than the patch I
> sent for that as well? (Perhaps my proposed fix was not completed, I
> just want to make sure the patch got noticed.)

Hm, I don't remember seeing it and can't find it now. Can you resend,
please?

I do see a previous patch that did miss from you, about a crypto+mime
fix. Is that still something that should be applied? It looks good to
me.

Anyways, I just merged my hook changes into next, on the branch
hook-local-vars. Take a look and tell me what you think. This should
also fix Edward Z. Yang's problems with hook locals not being useable in
statements like "x = x.foo()".
-- 
William <wmorgan-sup at masanjin.net>


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

* [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset
  2009-08-23 17:34             ` William Morgan
@ 2009-09-13 23:17               ` Adeodato Simó
  2009-09-26 18:12                 ` William Morgan
  0 siblings, 1 reply; 11+ messages in thread
From: Adeodato Simó @ 2009-09-13 23:17 UTC (permalink / raw)


+ William Morgan (Sun, 23 Aug 2009 10:34:10 -0700):

> Anyways, I just merged my hook changes into next, on the branch
> hook-local-vars. Take a look and tell me what you think. This should
> also fix Edward Z. Yang's problems with hook locals not being useable in
> statements like "x = x.foo()".

Works for me, thanks.

Now that that fix is in, can you merge the mime-decode improvement from
<6fc2e5dd8aa2b0b8547375d77b1776d779f85817.1247238014.git.dato at net.com.org.es>?
Thanks!

-- 
- Are you sure we're good?
- Always.
        -- Rory and Lorelai



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

* [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset
  2009-09-13 23:17               ` Adeodato Simó
@ 2009-09-26 18:12                 ` William Morgan
  0 siblings, 0 replies; 11+ messages in thread
From: William Morgan @ 2009-09-26 18:12 UTC (permalink / raw)


Reformatted excerpts from Adeodato Sim?'s message of 2009-09-13:
> Now that that fix is in, can you merge the mime-decode improvement from
> <6fc2e5dd8aa2b0b8547375d77b1776d779f85817.1247238014.git.dato at net.com.org.es>?

Applied directly to master. Sorry for the grand delay.
-- 
William <wmorgan-sup at masanjin.net>


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

end of thread, other threads:[~2009-09-26 18:12 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-10 15:00 [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset Adeodato Simó
2009-07-23  9:35 ` Adeodato Simó
2009-07-27 16:51   ` William Morgan
2009-07-30 15:56     ` Adeodato Simó
2009-07-30 15:57       ` [sup-talk] [PATCH] HookContext::method_missing(): allow locals to be nil Adeodato Simó
2009-08-19 21:22       ` [sup-talk] [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset Adeodato Simó
2009-08-22 18:15         ` William Morgan
2009-08-22 19:16           ` Adeodato Simó
2009-08-23 17:34             ` William Morgan
2009-09-13 23:17               ` Adeodato Simó
2009-09-26 18:12                 ` William Morgan

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