unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* Reply code with TEXT/PLAIN
@ 2012-03-24 21:49 Mark Walters
  2012-03-25  0:19 ` Adam Wolfe Gordon
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Walters @ 2012-03-24 21:49 UTC (permalink / raw)
  To: Adam Wolfe Gordon, notmuch



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

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

* Re: Reply code with TEXT/PLAIN
  2012-03-24 21:49 Reply code with TEXT/PLAIN Mark Walters
@ 2012-03-25  0:19 ` Adam Wolfe Gordon
  2012-03-25  0:43   ` [PATCH] emacs: content-type comparison should be case insensitive Mark Walters
  2012-03-26 15:21   ` Reply code with TEXT/PLAIN Jameson Graef Rollins
  0 siblings, 2 replies; 9+ messages in thread
From: Adam Wolfe Gordon @ 2012-03-25  0:19 UTC (permalink / raw)
  To: Mark Walters; +Cc: notmuch

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.

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

* [PATCH] emacs: content-type comparison should be case insensitive.
  2012-03-25  0:19 ` Adam Wolfe Gordon
@ 2012-03-25  0:43   ` Mark Walters
  2012-03-26 15:21     ` Jameson Graef Rollins
                       ` (2 more replies)
  2012-03-26 15:21   ` Reply code with TEXT/PLAIN Jameson Graef Rollins
  1 sibling, 3 replies; 9+ messages in thread
From: Mark Walters @ 2012-03-25  0:43 UTC (permalink / raw)
  To: notmuch

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

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

* Re: Reply code with TEXT/PLAIN
  2012-03-25  0:19 ` Adam Wolfe Gordon
  2012-03-25  0:43   ` [PATCH] emacs: content-type comparison should be case insensitive Mark Walters
@ 2012-03-26 15:21   ` Jameson Graef Rollins
  2012-03-26 15:24     ` Daniel Kahn Gillmor
  1 sibling, 1 reply; 9+ messages in thread
From: Jameson Graef Rollins @ 2012-03-26 15:21 UTC (permalink / raw)
  To: Adam Wolfe Gordon, Mark Walters; +Cc: notmuch

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

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

* Re: [PATCH] emacs: content-type comparison should be case insensitive.
  2012-03-25  0:43   ` [PATCH] emacs: content-type comparison should be case insensitive Mark Walters
@ 2012-03-26 15:21     ` Jameson Graef Rollins
  2012-03-26 19:40     ` Tomi Ollila
  2012-03-31  0:30     ` David Bremner
  2 siblings, 0 replies; 9+ messages in thread
From: Jameson Graef Rollins @ 2012-03-26 15:21 UTC (permalink / raw)
  To: Mark Walters, notmuch

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

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

* Re: Reply code with TEXT/PLAIN
  2012-03-26 15:21   ` Reply code with TEXT/PLAIN Jameson Graef Rollins
@ 2012-03-26 15:24     ` Daniel Kahn Gillmor
  2012-03-26 15:39       ` Jameson Graef Rollins
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Kahn Gillmor @ 2012-03-26 15:24 UTC (permalink / raw)
  To: notmuch

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

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

* Re: Reply code with TEXT/PLAIN
  2012-03-26 15:24     ` Daniel Kahn Gillmor
@ 2012-03-26 15:39       ` Jameson Graef Rollins
  0 siblings, 0 replies; 9+ messages in thread
From: Jameson Graef Rollins @ 2012-03-26 15:39 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, notmuch

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

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

* Re: [PATCH] emacs: content-type comparison should be case insensitive.
  2012-03-25  0:43   ` [PATCH] emacs: content-type comparison should be case insensitive Mark Walters
  2012-03-26 15:21     ` Jameson Graef Rollins
@ 2012-03-26 19:40     ` Tomi Ollila
  2012-03-31  0:30     ` David Bremner
  2 siblings, 0 replies; 9+ messages in thread
From: Tomi Ollila @ 2012-03-26 19:40 UTC (permalink / raw)
  To: Mark Walters, notmuch

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

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

* Re: [PATCH] emacs: content-type comparison should be case insensitive.
  2012-03-25  0:43   ` [PATCH] emacs: content-type comparison should be case insensitive Mark Walters
  2012-03-26 15:21     ` Jameson Graef Rollins
  2012-03-26 19:40     ` Tomi Ollila
@ 2012-03-31  0:30     ` David Bremner
  2 siblings, 0 replies; 9+ messages in thread
From: David Bremner @ 2012-03-31  0:30 UTC (permalink / raw)
  To: Mark Walters, notmuch

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

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

end of thread, other threads:[~2012-03-31  0:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-24 21:49 Reply code with TEXT/PLAIN Mark Walters
2012-03-25  0:19 ` Adam Wolfe Gordon
2012-03-25  0:43   ` [PATCH] emacs: content-type comparison should be case insensitive Mark Walters
2012-03-26 15:21     ` Jameson Graef Rollins
2012-03-26 19:40     ` Tomi Ollila
2012-03-31  0:30     ` David Bremner
2012-03-26 15:21   ` Reply code with TEXT/PLAIN Jameson Graef Rollins
2012-03-26 15:24     ` Daniel Kahn Gillmor
2012-03-26 15:39       ` Jameson Graef Rollins

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).