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

From: [email-masked] (Adeodato Simó)
Date: Fri, 10 Jul 2009 17:00:29 +0200

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

---

From: [email-masked] (Adeodato Simó)
Date: Thu, 23 Jul 2009 11:35:43 +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

---

From: [email-masked] (William Morgan)
Date: Mon, 27 Jul 2009 09:51:17 -0700

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

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>

---

From: [email-masked] (Adeodato Simó)
Date: Thu, 30 Jul 2009 17:56:56 +0200

> 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

---

From: [email-masked] (Adeodato Simó)
Date: Thu, 30 Jul 2009 17:57:16 +0200

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

---

From: [email-masked] (Adeodato Simó)
Date: Wed, 19 Aug 2009 22:22:09 +0100

Ping?

---

From: [email-masked] (William Morgan)
Date: Sat, 22 Aug 2009 11:15:15 -0700

> 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>

---

From: [email-masked] (Adeodato Simó)
Date: Sat, 22 Aug 2009 20:16:17 +0100

> 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,

---

From: [email-masked] (William Morgan)
Date: Sun, 23 Aug 2009 10:34:10 -0700

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>

---

From: [email-masked] (Adeodato Simó)
Date: Mon, 14 Sep 2009 00:17:53 +0100

> 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!

---

From: [email-masked] (William Morgan)
Date: Sat, 26 Sep 2009 11:12:50 -0700

> <6fc2e5dd8aa2b0b8547375d77b1776d779f85817.1247238014.git.dato at net.com.org.es>?

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