sup talk - Jul 2009 - [PATCH] mime-decode hook: provide a "charset" variable with the attachment charset

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

+ 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

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>

+ 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

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

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

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>

+ 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

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>

+ 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

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>