Reply code with TEXT/PLAIN

Reply code with TEXT/PLAIN

[Mark Walters]

I am not certain if this is a bug or a request to work around broken
mailers. I tried replying to a message today (with recent git) and got
an empty message. I looked at the json output for reply and it contains
the message but the content type is TEXT/PLAIN rather than
text/plain.

This seems to mean it doesn't match in notmuch-match-content-type
(notmuch-lib.el) called from notmuch-mua-get-quotable-parts (in
notmuch-mua.el); and so does not get included.

Making the match case-insensitive `fixed' the problem.

I don't know whether content-types are allowed  to be upper-case but I
seem to have several mails where they are. I am also not sure whether
the correct fix is in the emacs code, or in the cli reply format
(i.e. perhaps the reply format should lower-case the content-type).

Best wishes

Mark

PS Sorry Adam for the duplicate: I sent from a wrong address (my fault
this time)!

Re: Reply code with TEXT/PLAIN

[Adam Wolfe Gordon]

Hi Mark,

On Sat, Mar 24, 2012 at 15:49, Mark Walters <markwalters1009@gmail.com> wrote:
> I am not certain if this is a bug or a request to work around broken
> mailers. I tried replying to a message today (with recent git) and got
> an empty message. I looked at the json output for reply and it contains
> the message but the content type is TEXT/PLAIN rather than
> text/plain.
>
> This seems to mean it doesn't match in notmuch-match-content-type
> (notmuch-lib.el) called from notmuch-mua-get-quotable-parts (in
> notmuch-mua.el); and so does not get included.
>
> Making the match case-insensitive `fixed' the problem.
>
> I don't know whether content-types are allowed  to be upper-case but I
> seem to have several mails where they are. I am also not sure whether
> the correct fix is in the emacs code, or in the cli reply format
> (i.e. perhaps the reply format should lower-case the content-type).

A bit of Googling indicates that MIME types should be case insensitive
(i.e. TEXT/PLAIN should match text/plain). Given this, I think it
makes sense to change the emacs function regardless of whether it
makes sense for the CLI to output them in lower-case.
notmuch-match-content-type was put in notmuch-lib.el instead of in
notmuch-mua.el because it was thought that it might become useful
elsewhere later, so having it compare mime types correctly ensures
that future uses don't need to sanitize input to it.

Since you've already made the change for yourself, do you want to send a patch?

Cheers.

[PATCH] emacs: content-type comparison should be case insensitive.

[Mark Walters]

The function notmuch-match-content-type was comparing content types
case sensitively. Fix it so it tests case insensitively.

This fixes a bug where emacs would not include any body when replying
to a message with content-type TEXT/PLAIN.
---
 emacs/notmuch-lib.el |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index c146748..a754de7 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -185,8 +185,9 @@ the user hasn't set this variable with the old or new value."
 	(st2 (notmuch-split-content-type t2)))
     (if (or (string= (cadr st1) "*")
 	    (string= (cadr st2) "*"))
-	(string= (car st1) (car st2))
-      (string= t1 t2))))
+	;; Comparison of content types should be case insensitive.
+	(string= (downcase (car st1)) (downcase (car st2)))
+      (string= (downcase t1) (downcase t2)))))
 
 (defvar notmuch-multipart/alternative-discouraged
   '(
-- 
1.7.9.1

Re: [PATCH] emacs: content-type comparison should be case insensitive.

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1122 bytes --]

On Sun, 25 Mar 2012 00:43:28 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> The function notmuch-match-content-type was comparing content types
> case sensitively. Fix it so it tests case insensitively.
> 
> This fixes a bug where emacs would not include any body when replying
> to a message with content-type TEXT/PLAIN.
> ---
>  emacs/notmuch-lib.el |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index c146748..a754de7 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -185,8 +185,9 @@ the user hasn't set this variable with the old or new value."
>  	(st2 (notmuch-split-content-type t2)))
>      (if (or (string= (cadr st1) "*")
>  	    (string= (cadr st2) "*"))
> -	(string= (car st1) (car st2))
> -      (string= t1 t2))))
> +	;; Comparison of content types should be case insensitive.
> +	(string= (downcase (car st1)) (downcase (car st2)))
> +      (string= (downcase t1) (downcase t2)))))
>  
>  (defvar notmuch-multipart/alternative-discouraged
>    '(

LGTM.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] emacs: content-type comparison should be case insensitive.

[Tomi Ollila]

On Sun, Mar 25 2012, Mark Walters <markwalters1009@gmail.com> wrote:

> The function notmuch-match-content-type was comparing content types
> case sensitively. Fix it so it tests case insensitively.
>
> This fixes a bug where emacs would not include any body when replying
> to a message with content-type TEXT/PLAIN.
> ---

+1

Tomi

>  emacs/notmuch-lib.el |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>

Re: [PATCH] emacs: content-type comparison should be case insensitive.

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> The function notmuch-match-content-type was comparing content types
> case sensitively. Fix it so it tests case insensitively.
>

pushed,

d

Re: Reply code with TEXT/PLAIN

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 710 bytes --]

On Sat, 24 Mar 2012 18:19:50 -0600, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> A bit of Googling indicates that MIME types should be case insensitive
> (i.e. TEXT/PLAIN should match text/plain). Given this, I think it
> makes sense to change the emacs function regardless of whether it
> makes sense for the CLI to output them in lower-case.

Does it make sense for the cli to canonicalize the content types to
always be lower case?  Or should it continue to just pass the content
type exactly as it appears in the original message?  Given that
consumers should parse it case insensitively, it maybe doesn't matter,
but it might also be nice for us to be consistent.  Maybe it's just ok
as it is.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: Reply code with TEXT/PLAIN

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

On 03/26/2012 11:21 AM, Jameson Graef Rollins wrote:
> Does it make sense for the cli to canonicalize the content types to
> always be lower case?  Or should it continue to just pass the content
> type exactly as it appears in the original message?  Given that
> consumers should parse it case insensitively, it maybe doesn't matter,
> but it might also be nice for us to be consistent.

i think "notmuch emits the content-type in the exact form contained in
the underlying message" is a good flavor of consistency.

	--dkg



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 1030 bytes --]

Re: Reply code with TEXT/PLAIN

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

On Mon, 26 Mar 2012 11:24:29 -0400, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> On 03/26/2012 11:21 AM, Jameson Graef Rollins wrote:
> > Does it make sense for the cli to canonicalize the content types to
> > always be lower case?  Or should it continue to just pass the content
> > type exactly as it appears in the original message?  Given that
> > consumers should parse it case insensitively, it maybe doesn't matter,
> > but it might also be nice for us to be consistent.
> 
> i think "notmuch emits the content-type in the exact form contained in
> the underlying message" is a good flavor of consistency.

Works for me.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]