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