[RFC] Content-Description when naming MIME parts in Emacs mode

[RFC] Content-Description when naming MIME parts in Emacs mode

[W. Trevor King]

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

On the rss2email list, Victor Orlikowski pointed out [1] that a number
of MUAs don't use the Subject header of attached message/rfc822 parts
to label multipart/digest subparts [2].  Instead, notmuch and several
other MUAs use the filename parameter [3] as a content hint [4].
Using the filename parameter seems more sane than diving into the
message/rfc822 part header, but that's still not what the filename
parameter was designed for.  It makes more sense to me to use the
message/rfc822 part's Content-Description header [5,6], falling back
on the filename parameter if Content-Description isn't set.

It's pretty easy to patch notmuch-show-insert-bodypart to do this:

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 1ac80ca..485c7d1 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -874,13 +874,16 @@ useful for quoting in replies)."
                         content-type))
          (nth (plist-get part :id))
          (beg (point))
+         (name (if (plist-get part :content-description)
+                   (plist-get part :content-description)
+                   (plist-get part :filename)))
          ;; Hide the part initially if HIDE is t.
          (show-part (not (equal hide t)))
          ;; We omit the part button for the first (or only) part if
          ;; this is text/plain, or HIDE is 'no-buttons.
          (button (unless (or (equal hide 'no-buttons)
                              (and (string= mime-type "text/plain") (<= nth 1)))
-                   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
+                   (notmuch-show-insert-part-header nth mime-type content-type name)))
          (content-beg (point)))
 
     ;; Store the computed mime-type for later use (e.g. by attachment handlers).

But that doesn't work, because :content-description doesn't exist in
the part plist.  I've looked through the source for a bit and can't
figure out where that part plist is coming from.  Is it loaded from
notmuch output in notmuch-show-build-buffer?  I assume that
information comes from the index, in which case I'd need to tweak
_index_mime_part in lib/index.cc to add the description.  Indexing
descriptions seems like a generally useful thing, even outside of my
digest usecase (e.g. search image/jpeg attachements with “genome” in
their description [6]).  However, adding a field to the schema is more
invasive than changing the Emacs mode's attachment formatting; I
thought I should check in here for feedback and advice before wading
in with my —a̶x̶e̶— scalpel ;).

Thoughs?
Trevor

[1]: http://article.gmane.org/gmane.mail.rss2email/211
[2]: Digests: http://tools.ietf.org/html/rfc2046#section-5.1.5
[3]: Filename: http://tools.ietf.org/search/rfc2183#section-2.3
[4]: Filename hint to notmuch-show-insert-part-header:
     http://git.notmuchmail.org/git/notmuch/blob/HEAD:/emacs/notmuch-show.el#l883
[5]: Content-Desciption:
     http://tools.ietf.org/html/rfc2045#section-8
[6]: Content-Description examples:
     http://tools.ietf.org/html/rfc2183#section-3

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

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

Re: [RFC] Content-Description when naming MIME parts in Emacs mode

[David Bremner]

"W. Trevor King" <wking@tremily.us> writes:

> But that doesn't work, because :content-description doesn't exist in
> the part plist.  I've looked through the source for a bit and can't
> figure out where that part plist is coming from.

This is the parsed output from the command line call

     "notmuch show --format=sexp $searchterms"

d

Re: [RFC] Content-Description when naming MIME parts in Emacs mode

[Mark Walters]

On Sun, 02 Feb 2014, "W. Trevor King" <wking@tremily.us> wrote:
> On the rss2email list, Victor Orlikowski pointed out [1] that a number
> of MUAs don't use the Subject header of attached message/rfc822 parts
> to label multipart/digest subparts [2].  Instead, notmuch and several
> other MUAs use the filename parameter [3] as a content hint [4].
> Using the filename parameter seems more sane than diving into the
> message/rfc822 part header, but that's still not what the filename
> parameter was designed for.  It makes more sense to me to use the
> message/rfc822 part's Content-Description header [5,6], falling back
> on the filename parameter if Content-Description isn't set.
>
> It's pretty easy to patch notmuch-show-insert-bodypart to do this:
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 1ac80ca..485c7d1 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -874,13 +874,16 @@ useful for quoting in replies)."
>                          content-type))
>           (nth (plist-get part :id))
>           (beg (point))
> +         (name (if (plist-get part :content-description)
> +                   (plist-get part :content-description)
> +                   (plist-get part :filename)))
>           ;; Hide the part initially if HIDE is t.
>           (show-part (not (equal hide t)))
>           ;; We omit the part button for the first (or only) part if
>           ;; this is text/plain, or HIDE is 'no-buttons.
>           (button (unless (or (equal hide 'no-buttons)
>                               (and (string= mime-type "text/plain") (<= nth 1)))
> -                   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
> +                   (notmuch-show-insert-part-header nth mime-type content-type name)))
>           (content-beg (point)))
>  
>      ;; Store the computed mime-type for later use (e.g. by attachment handlers).
>
> But that doesn't work, because :content-description doesn't exist in
> the part plist.  I've looked through the source for a bit and can't
> figure out where that part plist is coming from.  Is it loaded from
> notmuch output in notmuch-show-build-buffer?  I assume that
> information comes from the index, in which case I'd need to tweak
> _index_mime_part in lib/index.cc to add the description.  Indexing
> descriptions seems like a generally useful thing, even outside of my
> digest usecase (e.g. search image/jpeg attachements with “genome” in
> their description [6]).  However, adding a field to the schema is more
> invasive than changing the Emacs mode's attachment formatting; I
> thought I should check in here for feedback and advice before wading
> in with my —a̶x̶e̶— scalpel ;).

I think notmuch-show.c gets most of the header and related information
directly from the mail file not from the database directly. I think we
use gmime for that parsing so adding an extra output pair for
content-description there should be sufficient. (This is lines 655-700
or so notmuch-show.c)

I think the emacs side should be roughly as above: we would need to
check that the default filename offered when saving is still
correct. Stylistically I think 
+         (name (or (plist-get part :content-description)
+                   (plist-get part :filename)))

is a little nicer.

I think that the above all looks like a sensible change to notmuch so I
would expect something like the above to be acceptable in mainline (but
obviously that is not my call!)

Best wishes

Mark

>
> Thoughs?
> Trevor
>
> [1]: http://article.gmane.org/gmane.mail.rss2email/211
> [2]: Digests: http://tools.ietf.org/html/rfc2046#section-5.1.5
> [3]: Filename: http://tools.ietf.org/search/rfc2183#section-2.3
> [4]: Filename hint to notmuch-show-insert-part-header:
>      http://git.notmuchmail.org/git/notmuch/blob/HEAD:/emacs/notmuch-show.el#l883
> [5]: Content-Desciption:
>      http://tools.ietf.org/html/rfc2045#section-8
> [6]: Content-Description examples:
>      http://tools.ietf.org/html/rfc2183#section-3
>
> -- 
> This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
> For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 0/2]

[W. Trevor King]

On Mon, Feb 03, 2014 at 08:04:21AM +0000, Mark Walters wrote:
> I think notmuch-show.c gets most of the header and related
> information directly from the mail file not from the database
> directly. I think we use gmime for that parsing so adding an extra
> output pair for content-description there should be
> sufficient. (This is lines 655-700 or so notmuch-show.c)

Thanks, that's exactly what I needed :).

> I think the emacs side should be roughly as above: we would need to
> check that the default filename offered when saving is still
> correct.

notmuch-save-attachments is using:

  (assq 'filename disposition)

and it looks like 'name' in notmuch-show-insert-part-header is only
used for the button text.

> Stylistically I think
> +         (name (or (plist-get part :content-description)
> +                   (plist-get part :filename)))
>
> is a little nicer.

Thanks for clearing up my search-engine Lisp ;).  Fixed in these
patches.

Cheers,
Trevor

p.s. I checked a few earlier patches on the list and didn't see
reviewer CCs.  On the other hand, I didn't see any Reply-To headers
either, so I'm CCing my reviewers to be safe ;).  Let me know if the
convention is to not CC reviewers, and I'll restrict future
submissions to notmuch@.

W. Trevor King (2):
  notmuch-show: Add content-description output pair
  emacs: Prefer Content-Description over filename for part buttons

 NEWS                                               |  9 +++
 emacs/notmuch-show.el                              |  4 +-
 notmuch-show.c                                     |  8 +++
 test/T160-json.sh                                  |  4 +-
 test/T170-sexp.sh                                  |  4 +-
 test/T450-emacs-show.sh                            | 12 ++++
 test/corpus/cur/24:2,                              |  1 +
 .../notmuch-show-buttons-content-description       | 84 ++++++++++++++++++++++
 .../notmuch-show-buttons-filename                  | 74 +++++++++++++++++++
 9 files changed, 195 insertions(+), 5 deletions(-)
 create mode 100644 test/emacs-show.expected-output/notmuch-show-buttons-content-description
 create mode 100644 test/emacs-show.expected-output/notmuch-show-buttons-filename

-- 
1.8.5.2.8.g0f6c0d1

[PATCH 1/2] notmuch-show: Add content-description output pair

[W. Trevor King]

Parse and display the Content-Description header [1,2] so UIs
rendering message parts have easy access to description strings.
Extracting the value is a two-step process (extract [3] and decode
[4]).

[1]: http://tools.ietf.org/html/rfc2045#section-8
[2]: http://tools.ietf.org/html/rfc2183#section-3
[3]: https://developer.gnome.org/gmime/stable/GMimeObject.html#g-mime-object-get-header
[4]: https://developer.gnome.org/gmime/stable/gmime-gmime-utils.html#g-mime-utils-header-decode-text
---
 NEWS              | 2 ++
 notmuch-show.c    | 8 ++++++++
 test/T160-json.sh | 4 ++--
 test/T170-sexp.sh | 4 ++--
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 65679eb..0f7b1c8 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,8 @@ Command-Line Interface
 
   The old format is still available with `--format=sup`.
 
+`notmuch show` now includes envelope Content-Description headers.
+
 Notmuch 0.17 (2013-12-30)
 =========================
 
diff --git a/notmuch-show.c b/notmuch-show.c
index d416fbd..a604227 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -659,6 +659,9 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	GMIME_OBJECT (node->envelope_part) : node->part;
     GMimeContentType *content_type = g_mime_object_get_content_type (meta);
     const char *cid = g_mime_object_get_content_id (meta);
+    const char *encoded_description = g_mime_object_get_header (meta, "content-description");
+    const char *description = encoded_description ?
+	g_mime_utils_header_decode_text (encoded_description) : NULL;
     const char *filename = GMIME_IS_PART (node->part) ?
 	g_mime_part_get_filename (GMIME_PART (node->part)) : NULL;
     int nclose = 0;
@@ -692,6 +695,11 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	sp->string (sp, cid);
     }
 
+    if (description) {
+	sp->map_key (sp, "content-description");
+	sp->string (sp, description);
+    }
+
     if (filename) {
 	sp->map_key (sp, "filename");
 	sp->string (sp, filename);
diff --git a/test/T160-json.sh b/test/T160-json.sh
index c1cf649..53f40ef 100755
--- a/test/T160-json.sh
+++ b/test/T160-json.sh
@@ -41,14 +41,14 @@ id="json-show-inline-attachment-filename@notmuchmail.org"
 emacs_fcc_message \
     "$subject" \
     'This is a test message with inline attachment with a filename' \
-    "(mml-attach-file \"$TEST_DIRECTORY/README\" nil nil \"inline\")
+    "(mml-attach-file \"$TEST_DIRECTORY/README\" nil \"Test README\" \"inline\")
      (message-goto-eoh)
      (insert \"Message-ID: <$id>\n\")"
 output=$(notmuch show --format=json "id:$id")
 filename=$(notmuch search --output=files "id:$id")
 # Get length of README after base64-encoding, minus additional newline.
 attachment_length=$(( $(base64 $TEST_DIRECTORY/README | wc -c) - 1 ))
-test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"test_suite@notmuchmail.org\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, \"content-type\": \"text/plain\", \"content\": \"This is a test message with inline attachment with a filename\"}, {\"id\": 3, \"content-type\": \"application/octet-stream\", \"content-length\": $attachment_length, \"content-transfer-encoding\": \"base64\", \"filename\": \"README\"}]}]}, []]]]"
+test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite <test_suite@notmuchmail.org>\", \"To\": \"test_suite@notmuchmail.org\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +0000\"}, \"body\": [{\"id\": 1, \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, \"content-type\": \"text/plain\", \"content\": \"This is a test message with inline attachment with a filename\"}, {\"id\": 3, \"content-type\": \"application/octet-stream\", \"content-description\": \"Test README\", \"content-length\": $attachment_length, \"content-transfer-encoding\": \"base64\", \"filename\": \"README\"}]}]}, []]]]"
 
 test_begin_subtest "Search message: json, utf-8"
 add_message "[subject]=\"json-search-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-search-méssage\""
diff --git a/test/T170-sexp.sh b/test/T170-sexp.sh
index 667e319..1c087d6 100755
--- a/test/T170-sexp.sh
+++ b/test/T170-sexp.sh
@@ -32,14 +32,14 @@ id="sexp-show-inline-attachment-filename@notmuchmail.org"
 emacs_fcc_message \
     "$subject" \
     'This is a test message with inline attachment with a filename' \
-    "(mml-attach-file \"$TEST_DIRECTORY/README\" nil nil \"inline\")
+    "(mml-attach-file \"$TEST_DIRECTORY/README\" nil \"Test README\" \"inline\")
      (message-goto-eoh)
      (insert \"Message-ID: <$id>\n\")"
 output=$(notmuch show --format=sexp "id:$id")
 filename=$(notmuch search --output=files "id:$id")
 # Get length of README after base64-encoding, minus additional newline.
 attachment_length=$(( $(base64 $TEST_DIRECTORY/README | wc -c) - 1 ))
-test_expect_equal "$output" "((((:id \"$id\" :match t :excluded nil :filename \"$filename\" :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\") :headers (:Subject \"sexp-show-inline-attachment-filename\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"test_suite@notmuchmail.org\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\") :body ((:id 1 :content-type \"multipart/mixed\" :content ((:id 2 :content-type \"text/plain\" :content \"This is a test message with inline attachment with a filename\") (:id 3 :content-type \"application/octet-stream\" :filename \"README\" :content-transfer-encoding \"base64\" :content-length $attachment_length))))) ())))"
+test_expect_equal "$output" "((((:id \"$id\" :match t :excluded nil :filename \"$filename\" :timestamp 946728000 :date_relative \"2000-01-01\" :tags (\"inbox\") :headers (:Subject \"sexp-show-inline-attachment-filename\" :From \"Notmuch Test Suite <test_suite@notmuchmail.org>\" :To \"test_suite@notmuchmail.org\" :Date \"Sat, 01 Jan 2000 12:00:00 +0000\") :body ((:id 1 :content-type \"multipart/mixed\" :content ((:id 2 :content-type \"text/plain\" :content \"This is a test message with inline attachment with a filename\") (:id 3 :content-type \"application/octet-stream\" :content-description \"Test README\" :filename \"README\" :content-transfer-encoding \"base64\" :content-length $attachment_length))))) ())))"
 
 test_begin_subtest "Search message: sexp, utf-8"
 add_message "[subject]=\"sexp-search-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-search-méssage\""
-- 
1.8.5.2.8.g0f6c0d1

[PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[W. Trevor King]

On the rss2email list, Victor Orlikowski pointed out [1] that a number
of MUAs don't use the Subject header of attached message/rfc822 parts
to label multipart/digest subparts [2].  Instead, notmuch and several
other MUAs use the filename parameter [3] as a content hint.  Using
the filename parameter seems more sane than diving into the
message/rfc822 part header, but that's still not what the filename
parameter was designed for.  It makes more sense to me to use the
message/rfc822 part's Content-Description header (which I just taught
notmuch-show to export), falling back on the filename parameter if
Content-Description isn't set.

[1]: http://article.gmane.org/gmane.mail.rss2email/211
[2]: Digests: http://tools.ietf.org/html/rfc2046#section-5.1.5
[3]: Filename: http://tools.ietf.org/search/rfc2183#section-2.3
---
 NEWS                                               |  7 ++
 emacs/notmuch-show.el                              |  4 +-
 test/T450-emacs-show.sh                            | 12 ++++
 test/corpus/cur/24:2,                              |  1 +
 .../notmuch-show-buttons-content-description       | 84 ++++++++++++++++++++++
 .../notmuch-show-buttons-filename                  | 74 +++++++++++++++++++
 6 files changed, 181 insertions(+), 1 deletion(-)
 create mode 100644 test/emacs-show.expected-output/notmuch-show-buttons-content-description
 create mode 100644 test/emacs-show.expected-output/notmuch-show-buttons-filename

diff --git a/NEWS b/NEWS
index 0f7b1c8..2154872 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,13 @@ Command-Line Interface
 
 `notmuch show` now includes envelope Content-Description headers.
 
+Emacs Interface
+---------------
+
+`notmuch-show` mode prefers Content-Description to filename when
+naming part buttons.  This is useful for finding interesting parts of
+multipart/digest messages, assuming the digest-creator set that field.
+
 Notmuch 0.17 (2013-12-30)
 =========================
 
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 1ac80ca..dbff3a8 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -874,13 +874,15 @@ useful for quoting in replies)."
 			content-type))
 	 (nth (plist-get part :id))
 	 (beg (point))
+	 (name (or (plist-get part :content-description)
+		   (plist-get part :filename)))
 	 ;; Hide the part initially if HIDE is t.
 	 (show-part (not (equal hide t)))
 	 ;; We omit the part button for the first (or only) part if
 	 ;; this is text/plain, or HIDE is 'no-buttons.
 	 (button (unless (or (equal hide 'no-buttons)
 			     (and (string= mime-type "text/plain") (<= nth 1)))
-		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
+		   (notmuch-show-insert-part-header nth mime-type content-type name)))
 	 (content-beg (point)))
 
     ;; Store the computed mime-type for later use (e.g. by attachment handlers).
diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
index 2a3a535..5650f04 100755
--- a/test/T450-emacs-show.sh
+++ b/test/T450-emacs-show.sh
@@ -106,6 +106,18 @@ test_emacs '(notmuch-search "from:lars@seas.harvard.edu and subject:\"Maildir st
 	(test-visible-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-indent-thread-content-off
 
+test_begin_subtest "buttons prefer Content-Description"
+test_emacs '(let ((notmuch-crypto-process-mime nil))
+	(notmuch-show "id:20091118010116.GC25380@dottiness.seas.harvard.edu")
+	(test-visible-output))'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-buttons-content-description
+
+test_begin_subtest "buttons fallback to filename"
+test_emacs '(let ((notmuch-crypto-process-mime nil))
+	(notmuch-show "id:20091118005829.GB25380@dottiness.seas.harvard.edu")
+	(test-visible-output))'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-buttons-filename
+
 test_begin_subtest "id buttonization"
 add_message '[body]="
 id:abc
diff --git a/test/corpus/cur/24:2, b/test/corpus/cur/24:2,
index c800020..f9418d2 100644
--- a/test/corpus/cur/24:2,
+++ b/test/corpus/cur/24:2,
@@ -104,6 +104,7 @@ Harvard University School of Engineering and Applied Sciences
 
 --KdquIMZPjGJQvRdI
 Content-Type: text/plain; charset=us-ascii
+Content-Description: v2 of the selectable usage() stream patch
 Content-Disposition: attachment; filename="notmuch-help.patch"
 Content-Transfer-Encoding: quoted-printable
 
diff --git a/test/emacs-show.expected-output/notmuch-show-buttons-content-description b/test/emacs-show.expected-output/notmuch-show-buttons-content-description
new file mode 100644
index 0000000..622c94e
--- /dev/null
+++ b/test/emacs-show.expected-output/notmuch-show-buttons-content-description
@@ -0,0 +1,84 @@
+Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed unread)
+Subject: [notmuch] "notmuch help" outputs to stderr?
+ Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed)
+ Subject: Re: [notmuch] "notmuch help" outputs to stderr?
+ To: notmuch <notmuch@notmuchmail.org>
+ Date: Tue, 17 Nov 2009 20:01:16 -0500
+
+ [ multipart/mixed ]
+ [ multipart/signed ]
+ [ multipart/mixed ]
+ [ text/plain ]
+ > I've attached a patch that lets usage() take a FILE * argument so that
+ > you can output to stderr in response to usage errors, and stdout in
+ > response to an explicit request.
+
+ Whoops, missed a couple of stderr's in that last patch.  New one
+ attached.
+
+ [ 4-line signature. Click/Enter to show. ]
+ [ v2 of the selectable usage() stream patch: text/plain ]
+ diff --git a/notmuch.c b/notmuch.c
+ index c47e640..446c810 100644
+ --- a/notmuch.c
+ +++ b/notmuch.c
+ @@ -157,23 +157,23 @@ command_t commands[] = {
+  };
+
+  static void
+ -usage (void)
+ +usage (FILE *out)
+  {
+      command_t *command;
+      unsigned int i;
+
+ -    fprintf (stderr, "Usage: notmuch <command> [args...]\n");
+ -    fprintf (stderr, "\n");
+ -    fprintf (stderr, "Where <command> and [args...] are as follows:\n");
+ -    fprintf (stderr, "\n");
+ +    fprintf (out, "Usage: notmuch <command> [args...]\n");
+ +    fprintf (out, "\n");
+ +    fprintf (out, "Where <command> and [args...] are as follows:\n");
+ +    fprintf (out, "\n");
+
+      for (i = 0; i < ARRAY_SIZE (commands); i++) {
+	 command = &commands[i];
+
+ -	fprintf (stderr, "\t%s\t%s\n\n", command->name, command->summary);
+ +	fprintf (out, "\t%s\t%s\n\n", command->name, command->summary);
+      }
+
+ -    fprintf (stderr, "Use \"notmuch help <command>\" for more details on
+ each command.\n\n");
+ +    fprintf (out, "Use \"notmuch help <command>\" for more details on each
+ command.\n\n");
+  }
+
+  static int
+ @@ -183,8 +183,8 @@ notmuch_help_command (unused (void *ctx), int argc, char
+ *argv[])
+      unsigned int i;
+
+      if (argc == 0) {
+ -	fprintf (stderr, "The notmuch mail system.\n\n");
+ -	usage ();
+ +	fprintf (stdout, "The notmuch mail system.\n\n");
+ +	usage (stdout);
+	 return 0;
+      }
+
+ @@ -192,8 +192,8 @@ notmuch_help_command (unused (void *ctx), int argc, char
+ *argv[])
+	 command = &commands[i];
+
+	 if (strcmp (argv[0], command->name) == 0) {
+ -	    fprintf (stderr, "Help for \"notmuch %s\":\n\n", argv[0]);
+ -	    fprintf (stderr, "\t%s\t%s\n\n%s\n\n", command->name,
+ +	    fprintf (stdout, "Help for \"notmuch %s\":\n\n", argv[0]);
+ +	    fprintf (stdout, "\t%s\t%s\n\n%s\n\n", command->name,
+		      command->summary, command->documentation);
+	     return 0;
+	 }
+ [ application/pgp-signature ]
+ [ text/plain ]
+ [ 4-line signature. Click/Enter to show. ]
diff --git a/test/emacs-show.expected-output/notmuch-show-buttons-filename b/test/emacs-show.expected-output/notmuch-show-buttons-filename
new file mode 100644
index 0000000..8fb4d89
--- /dev/null
+++ b/test/emacs-show.expected-output/notmuch-show-buttons-filename
@@ -0,0 +1,74 @@
+Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed)
+Subject: [notmuch] "notmuch help" outputs to stderr?
+To: notmuch <notmuch@notmuchmail.org>
+Date: Tue, 17 Nov 2009 19:58:29 -0500
+
+[ multipart/mixed ]
+[ multipart/signed ]
+[ multipart/mixed ]
+[ text/plain ]
+I'm just noticing that 'notmuch help ...' outputs to stderr, which
+isn't terribly intuitive.  For example, the obvious invocation:
+
+  notmuch help | less
+
+...isn't terribly helpful.
+
+I've attached a patch that lets usage() take a FILE * argument so that
+you can output to stderr in response to usage errors, and stdout in
+response to an explicit request.
+
+[ 4-line signature. Click/Enter to show. ]
+[ notmuch-help.patch: text/plain ]
+diff --git a/notmuch.c b/notmuch.c
+index c47e640..a35cb99 100644
+--- a/notmuch.c
++++ b/notmuch.c
+@@ -157,23 +157,23 @@ command_t commands[] = {
+ };
+
+ static void
+-usage (void)
++usage (FILE *out)
+ {
+     command_t *command;
+     unsigned int i;
+
+-    fprintf (stderr, "Usage: notmuch <command> [args...]\n");
+-    fprintf (stderr, "\n");
+-    fprintf (stderr, "Where <command> and [args...] are as follows:\n");
+-    fprintf (stderr, "\n");
++    fprintf (out, "Usage: notmuch <command> [args...]\n");
++    fprintf (out, "\n");
++    fprintf (out, "Where <command> and [args...] are as follows:\n");
++    fprintf (out, "\n");
+
+     for (i = 0; i < ARRAY_SIZE (commands); i++) {
+	command = &commands[i];
+
+-	fprintf (stderr, "\t%s\t%s\n\n", command->name, command->summary);
++	fprintf (out, "\t%s\t%s\n\n", command->name, command->summary);
+     }
+
+-    fprintf (stderr, "Use \"notmuch help <command>\" for more details on each
+command.\n\n");
++    fprintf (out, "Use \"notmuch help <command>\" for more details on each
+command.\n\n");
+ }
+
+ static int
+@@ -183,8 +183,8 @@ notmuch_help_command (unused (void *ctx), int argc, char
+*argv[])
+     unsigned int i;
+
+     if (argc == 0) {
+-	fprintf (stderr, "The notmuch mail system.\n\n");
+-	usage ();
++	fprintf (stdout, "The notmuch mail system.\n\n");
++	usage (stdout);
+	return 0;
+     }
+[ application/pgp-signature ]
+[ text/plain ]
+[ 4-line signature. Click/Enter to show. ]
+ Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed)
-- 
1.8.5.2.8.g0f6c0d1

Re: [PATCH 0/2]

[David Bremner]

"W. Trevor King" <wking@tremily.us> writes:

>
> p.s. I checked a few earlier patches on the list and didn't see
> reviewer CCs.  On the other hand, I didn't see any Reply-To headers
> either, so I'm CCing my reviewers to be safe ;).  Let me know if the
> convention is to not CC reviewers, and I'll restrict future
> submissions to notmuch@.

I would say this list is CC friendly. Most (?) of us read it with
notmuch, and don't see the CC's as extra messages.

d

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[Mark Walters]

On Mon, 03 Feb 2014, "W. Trevor King" <wking@tremily.us> wrote:
> On the rss2email list, Victor Orlikowski pointed out [1] that a number
> of MUAs don't use the Subject header of attached message/rfc822 parts
> to label multipart/digest subparts [2].  Instead, notmuch and several
> other MUAs use the filename parameter [3] as a content hint.  Using
> the filename parameter seems more sane than diving into the
> message/rfc822 part header, but that's still not what the filename
> parameter was designed for.  It makes more sense to me to use the
> message/rfc822 part's Content-Description header (which I just taught
> notmuch-show to export), falling back on the filename parameter if
> Content-Description isn't set.
>
> [1]: http://article.gmane.org/gmane.mail.rss2email/211
> [2]: Digests: http://tools.ietf.org/html/rfc2046#section-5.1.5
> [3]: Filename: http://tools.ietf.org/search/rfc2183#section-2.3

I tried this and it all works. However, I looked through my collection
an I have a lot of emails which have Content-Description headers that
are empty. Thus, I think we should only display/use the
content-description if it exists and is non-empty. Something like

	 (description (plist-get part :content-description))
	 (name (if (and description (not (equal description "")))
		   description
		 (plist-get part :filename)))

My only other comment on the series is that you should update
devel/schemata in the first patch to reflect this addition.

Otherwise it looks good to me

Best wishes

Mark


> ---
>  NEWS                                               |  7 ++
>  emacs/notmuch-show.el                              |  4 +-
>  test/T450-emacs-show.sh                            | 12 ++++
>  test/corpus/cur/24:2,                              |  1 +
>  .../notmuch-show-buttons-content-description       | 84 ++++++++++++++++++++++
>  .../notmuch-show-buttons-filename                  | 74 +++++++++++++++++++
>  6 files changed, 181 insertions(+), 1 deletion(-)
>  create mode 100644 test/emacs-show.expected-output/notmuch-show-buttons-content-description
>  create mode 100644 test/emacs-show.expected-output/notmuch-show-buttons-filename
>
> diff --git a/NEWS b/NEWS
> index 0f7b1c8..2154872 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,13 @@ Command-Line Interface
>  
>  `notmuch show` now includes envelope Content-Description headers.
>  
> +Emacs Interface
> +---------------
> +
> +`notmuch-show` mode prefers Content-Description to filename when
> +naming part buttons.  This is useful for finding interesting parts of
> +multipart/digest messages, assuming the digest-creator set that field.
> +
>  Notmuch 0.17 (2013-12-30)
>  =========================
>  
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 1ac80ca..dbff3a8 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -874,13 +874,15 @@ useful for quoting in replies)."
>  			content-type))
>  	 (nth (plist-get part :id))
>  	 (beg (point))
> +	 (name (or (plist-get part :content-description)
> +		   (plist-get part :filename)))
>  	 ;; Hide the part initially if HIDE is t.
>  	 (show-part (not (equal hide t)))
>  	 ;; We omit the part button for the first (or only) part if
>  	 ;; this is text/plain, or HIDE is 'no-buttons.
>  	 (button (unless (or (equal hide 'no-buttons)
>  			     (and (string= mime-type "text/plain") (<= nth 1)))
> -		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
> +		   (notmuch-show-insert-part-header nth mime-type content-type name)))
>  	 (content-beg (point)))
>  
>      ;; Store the computed mime-type for later use (e.g. by attachment handlers).
> diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
> index 2a3a535..5650f04 100755
> --- a/test/T450-emacs-show.sh
> +++ b/test/T450-emacs-show.sh
> @@ -106,6 +106,18 @@ test_emacs '(notmuch-search "from:lars@seas.harvard.edu and subject:\"Maildir st
>  	(test-visible-output)'
>  test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-indent-thread-content-off
>  
> +test_begin_subtest "buttons prefer Content-Description"
> +test_emacs '(let ((notmuch-crypto-process-mime nil))
> +	(notmuch-show "id:20091118010116.GC25380@dottiness.seas.harvard.edu")
> +	(test-visible-output))'
> +test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-buttons-content-description
> +
> +test_begin_subtest "buttons fallback to filename"
> +test_emacs '(let ((notmuch-crypto-process-mime nil))
> +	(notmuch-show "id:20091118005829.GB25380@dottiness.seas.harvard.edu")
> +	(test-visible-output))'
> +test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-buttons-filename
> +
>  test_begin_subtest "id buttonization"
>  add_message '[body]="
>  id:abc
> diff --git a/test/corpus/cur/24:2, b/test/corpus/cur/24:2,
> index c800020..f9418d2 100644
> --- a/test/corpus/cur/24:2,
> +++ b/test/corpus/cur/24:2,
> @@ -104,6 +104,7 @@ Harvard University School of Engineering and Applied Sciences
>  
>  --KdquIMZPjGJQvRdI
>  Content-Type: text/plain; charset=us-ascii
> +Content-Description: v2 of the selectable usage() stream patch
>  Content-Disposition: attachment; filename="notmuch-help.patch"
>  Content-Transfer-Encoding: quoted-printable
>  
> diff --git a/test/emacs-show.expected-output/notmuch-show-buttons-content-description b/test/emacs-show.expected-output/notmuch-show-buttons-content-description
> new file mode 100644
> index 0000000..622c94e
> --- /dev/null
> +++ b/test/emacs-show.expected-output/notmuch-show-buttons-content-description
> @@ -0,0 +1,84 @@
> +Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed unread)
> +Subject: [notmuch] "notmuch help" outputs to stderr?
> + Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed)
> + Subject: Re: [notmuch] "notmuch help" outputs to stderr?
> + To: notmuch <notmuch@notmuchmail.org>
> + Date: Tue, 17 Nov 2009 20:01:16 -0500
> +
> + [ multipart/mixed ]
> + [ multipart/signed ]
> + [ multipart/mixed ]
> + [ text/plain ]
> + > I've attached a patch that lets usage() take a FILE * argument so that
> + > you can output to stderr in response to usage errors, and stdout in
> + > response to an explicit request.
> +
> + Whoops, missed a couple of stderr's in that last patch.  New one
> + attached.
> +
> + [ 4-line signature. Click/Enter to show. ]
> + [ v2 of the selectable usage() stream patch: text/plain ]
> + diff --git a/notmuch.c b/notmuch.c
> + index c47e640..446c810 100644
> + --- a/notmuch.c
> + +++ b/notmuch.c
> + @@ -157,23 +157,23 @@ command_t commands[] = {
> +  };
> +
> +  static void
> + -usage (void)
> + +usage (FILE *out)
> +  {
> +      command_t *command;
> +      unsigned int i;
> +
> + -    fprintf (stderr, "Usage: notmuch <command> [args...]\n");
> + -    fprintf (stderr, "\n");
> + -    fprintf (stderr, "Where <command> and [args...] are as follows:\n");
> + -    fprintf (stderr, "\n");
> + +    fprintf (out, "Usage: notmuch <command> [args...]\n");
> + +    fprintf (out, "\n");
> + +    fprintf (out, "Where <command> and [args...] are as follows:\n");
> + +    fprintf (out, "\n");
> +
> +      for (i = 0; i < ARRAY_SIZE (commands); i++) {
> +	 command = &commands[i];
> +
> + -	fprintf (stderr, "\t%s\t%s\n\n", command->name, command->summary);
> + +	fprintf (out, "\t%s\t%s\n\n", command->name, command->summary);
> +      }
> +
> + -    fprintf (stderr, "Use \"notmuch help <command>\" for more details on
> + each command.\n\n");
> + +    fprintf (out, "Use \"notmuch help <command>\" for more details on each
> + command.\n\n");
> +  }
> +
> +  static int
> + @@ -183,8 +183,8 @@ notmuch_help_command (unused (void *ctx), int argc, char
> + *argv[])
> +      unsigned int i;
> +
> +      if (argc == 0) {
> + -	fprintf (stderr, "The notmuch mail system.\n\n");
> + -	usage ();
> + +	fprintf (stdout, "The notmuch mail system.\n\n");
> + +	usage (stdout);
> +	 return 0;
> +      }
> +
> + @@ -192,8 +192,8 @@ notmuch_help_command (unused (void *ctx), int argc, char
> + *argv[])
> +	 command = &commands[i];
> +
> +	 if (strcmp (argv[0], command->name) == 0) {
> + -	    fprintf (stderr, "Help for \"notmuch %s\":\n\n", argv[0]);
> + -	    fprintf (stderr, "\t%s\t%s\n\n%s\n\n", command->name,
> + +	    fprintf (stdout, "Help for \"notmuch %s\":\n\n", argv[0]);
> + +	    fprintf (stdout, "\t%s\t%s\n\n%s\n\n", command->name,
> +		      command->summary, command->documentation);
> +	     return 0;
> +	 }
> + [ application/pgp-signature ]
> + [ text/plain ]
> + [ 4-line signature. Click/Enter to show. ]
> diff --git a/test/emacs-show.expected-output/notmuch-show-buttons-filename b/test/emacs-show.expected-output/notmuch-show-buttons-filename
> new file mode 100644
> index 0000000..8fb4d89
> --- /dev/null
> +++ b/test/emacs-show.expected-output/notmuch-show-buttons-filename
> @@ -0,0 +1,74 @@
> +Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed)
> +Subject: [notmuch] "notmuch help" outputs to stderr?
> +To: notmuch <notmuch@notmuchmail.org>
> +Date: Tue, 17 Nov 2009 19:58:29 -0500
> +
> +[ multipart/mixed ]
> +[ multipart/signed ]
> +[ multipart/mixed ]
> +[ text/plain ]
> +I'm just noticing that 'notmuch help ...' outputs to stderr, which
> +isn't terribly intuitive.  For example, the obvious invocation:
> +
> +  notmuch help | less
> +
> +...isn't terribly helpful.
> +
> +I've attached a patch that lets usage() take a FILE * argument so that
> +you can output to stderr in response to usage errors, and stdout in
> +response to an explicit request.
> +
> +[ 4-line signature. Click/Enter to show. ]
> +[ notmuch-help.patch: text/plain ]
> +diff --git a/notmuch.c b/notmuch.c
> +index c47e640..a35cb99 100644
> +--- a/notmuch.c
> ++++ b/notmuch.c
> +@@ -157,23 +157,23 @@ command_t commands[] = {
> + };
> +
> + static void
> +-usage (void)
> ++usage (FILE *out)
> + {
> +     command_t *command;
> +     unsigned int i;
> +
> +-    fprintf (stderr, "Usage: notmuch <command> [args...]\n");
> +-    fprintf (stderr, "\n");
> +-    fprintf (stderr, "Where <command> and [args...] are as follows:\n");
> +-    fprintf (stderr, "\n");
> ++    fprintf (out, "Usage: notmuch <command> [args...]\n");
> ++    fprintf (out, "\n");
> ++    fprintf (out, "Where <command> and [args...] are as follows:\n");
> ++    fprintf (out, "\n");
> +
> +     for (i = 0; i < ARRAY_SIZE (commands); i++) {
> +	command = &commands[i];
> +
> +-	fprintf (stderr, "\t%s\t%s\n\n", command->name, command->summary);
> ++	fprintf (out, "\t%s\t%s\n\n", command->name, command->summary);
> +     }
> +
> +-    fprintf (stderr, "Use \"notmuch help <command>\" for more details on each
> +command.\n\n");
> ++    fprintf (out, "Use \"notmuch help <command>\" for more details on each
> +command.\n\n");
> + }
> +
> + static int
> +@@ -183,8 +183,8 @@ notmuch_help_command (unused (void *ctx), int argc, char
> +*argv[])
> +     unsigned int i;
> +
> +     if (argc == 0) {
> +-	fprintf (stderr, "The notmuch mail system.\n\n");
> +-	usage ();
> ++	fprintf (stdout, "The notmuch mail system.\n\n");
> ++	usage (stdout);
> +	return 0;
> +     }
> +[ application/pgp-signature ]
> +[ text/plain ]
> +[ 4-line signature. Click/Enter to show. ]
> + Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed)
> -- 
> 1.8.5.2.8.g0f6c0d1

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[W. Trevor King]

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

On Mon, Feb 03, 2014 at 08:15:35PM +0000, Mark Walters wrote:
> I think we should only display/use the content-description if it
> exists and is non-empty.

Sounds good to me.  I'll queue this and a test for v2.

> My only other comment on the series is that you should update
> devel/schemata in the first patch to reflect this addition.

Ah, I didn't realize that file existed :p.  Also queued for v2.  I'll
let this cook for a bit longer, and resubmit if there is no further
feedback.

Thanks,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

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

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[Tomi Ollila]

On Mon, Feb 03 2014, "W. Trevor King" <wking@tremily.us> wrote:

> On the rss2email list, Victor Orlikowski pointed out [1] that a number
> of MUAs don't use the Subject header of attached message/rfc822 parts
> to label multipart/digest subparts [2].  Instead, notmuch and several
> other MUAs use the filename parameter [3] as a content hint.  Using
> the filename parameter seems more sane than diving into the
> message/rfc822 part header, but that's still not what the filename
> parameter was designed for.  It makes more sense to me to use the
> message/rfc822 part's Content-Description header (which I just taught
> notmuch-show to export), falling back on the filename parameter if
> Content-Description isn't set.

Series looks good, although having notmuch patch email as expected test
output feels a bit confusing to me -- especially as 'git grep' may
catch some of the (possibly future-outdated) content...

Tomi


>
> [1]: http://article.gmane.org/gmane.mail.rss2email/211
> [2]: Digests: http://tools.ietf.org/html/rfc2046#section-5.1.5
> [3]: Filename: http://tools.ietf.org/search/rfc2183#section-2.3
> ---
>  NEWS                                               |  7 ++
>  emacs/notmuch-show.el                              |  4 +-
>  test/T450-emacs-show.sh                            | 12 ++++
>  test/corpus/cur/24:2,                              |  1 +
>  .../notmuch-show-buttons-content-description       | 84 ++++++++++++++++++++++
>  .../notmuch-show-buttons-filename                  | 74 +++++++++++++++++++
>  6 files changed, 181 insertions(+), 1 deletion(-)
>  create mode 100644 test/emacs-show.expected-output/notmuch-show-buttons-content-description
>  create mode 100644 test/emacs-show.expected-output/notmuch-show-buttons-filename
>
> diff --git a/NEWS b/NEWS
> index 0f7b1c8..2154872 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,13 @@ Command-Line Interface
>  
>  `notmuch show` now includes envelope Content-Description headers.
>  
> +Emacs Interface
> +---------------
> +
> +`notmuch-show` mode prefers Content-Description to filename when
> +naming part buttons.  This is useful for finding interesting parts of
> +multipart/digest messages, assuming the digest-creator set that field.
> +
>  Notmuch 0.17 (2013-12-30)
>  =========================
>  
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 1ac80ca..dbff3a8 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -874,13 +874,15 @@ useful for quoting in replies)."
>  			content-type))
>  	 (nth (plist-get part :id))
>  	 (beg (point))
> +	 (name (or (plist-get part :content-description)
> +		   (plist-get part :filename)))
>  	 ;; Hide the part initially if HIDE is t.
>  	 (show-part (not (equal hide t)))
>  	 ;; We omit the part button for the first (or only) part if
>  	 ;; this is text/plain, or HIDE is 'no-buttons.
>  	 (button (unless (or (equal hide 'no-buttons)
>  			     (and (string= mime-type "text/plain") (<= nth 1)))
> -		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename))))
> +		   (notmuch-show-insert-part-header nth mime-type content-type name)))
>  	 (content-beg (point)))
>  
>      ;; Store the computed mime-type for later use (e.g. by attachment handlers).
> diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
> index 2a3a535..5650f04 100755
> --- a/test/T450-emacs-show.sh
> +++ b/test/T450-emacs-show.sh
> @@ -106,6 +106,18 @@ test_emacs '(notmuch-search "from:lars@seas.harvard.edu and subject:\"Maildir st
>  	(test-visible-output)'
>  test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-indent-thread-content-off
>  
> +test_begin_subtest "buttons prefer Content-Description"
> +test_emacs '(let ((notmuch-crypto-process-mime nil))
> +	(notmuch-show "id:20091118010116.GC25380@dottiness.seas.harvard.edu")
> +	(test-visible-output))'
> +test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-buttons-content-description
> +
> +test_begin_subtest "buttons fallback to filename"
> +test_emacs '(let ((notmuch-crypto-process-mime nil))
> +	(notmuch-show "id:20091118005829.GB25380@dottiness.seas.harvard.edu")
> +	(test-visible-output))'
> +test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-buttons-filename
> +
>  test_begin_subtest "id buttonization"
>  add_message '[body]="
>  id:abc
> diff --git a/test/corpus/cur/24:2, b/test/corpus/cur/24:2,
> index c800020..f9418d2 100644
> --- a/test/corpus/cur/24:2,
> +++ b/test/corpus/cur/24:2,
> @@ -104,6 +104,7 @@ Harvard University School of Engineering and Applied Sciences
>  
>  --KdquIMZPjGJQvRdI
>  Content-Type: text/plain; charset=us-ascii
> +Content-Description: v2 of the selectable usage() stream patch
>  Content-Disposition: attachment; filename="notmuch-help.patch"
>  Content-Transfer-Encoding: quoted-printable
>  
> diff --git a/test/emacs-show.expected-output/notmuch-show-buttons-content-description b/test/emacs-show.expected-output/notmuch-show-buttons-content-description
> new file mode 100644
> index 0000000..622c94e
> --- /dev/null
> +++ b/test/emacs-show.expected-output/notmuch-show-buttons-content-description
> @@ -0,0 +1,84 @@
> +Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed unread)
> +Subject: [notmuch] "notmuch help" outputs to stderr?
> + Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed)
> + Subject: Re: [notmuch] "notmuch help" outputs to stderr?
> + To: notmuch <notmuch@notmuchmail.org>
> + Date: Tue, 17 Nov 2009 20:01:16 -0500
> +
> + [ multipart/mixed ]
> + [ multipart/signed ]
> + [ multipart/mixed ]
> + [ text/plain ]
> + > I've attached a patch that lets usage() take a FILE * argument so that
> + > you can output to stderr in response to usage errors, and stdout in
> + > response to an explicit request.
> +
> + Whoops, missed a couple of stderr's in that last patch.  New one
> + attached.
> +
> + [ 4-line signature. Click/Enter to show. ]
> + [ v2 of the selectable usage() stream patch: text/plain ]
> + diff --git a/notmuch.c b/notmuch.c
> + index c47e640..446c810 100644
> + --- a/notmuch.c
> + +++ b/notmuch.c
> + @@ -157,23 +157,23 @@ command_t commands[] = {
> +  };
> +
> +  static void
> + -usage (void)
> + +usage (FILE *out)
> +  {
> +      command_t *command;
> +      unsigned int i;
> +
> + -    fprintf (stderr, "Usage: notmuch <command> [args...]\n");
> + -    fprintf (stderr, "\n");
> + -    fprintf (stderr, "Where <command> and [args...] are as follows:\n");
> + -    fprintf (stderr, "\n");
> + +    fprintf (out, "Usage: notmuch <command> [args...]\n");
> + +    fprintf (out, "\n");
> + +    fprintf (out, "Where <command> and [args...] are as follows:\n");
> + +    fprintf (out, "\n");
> +
> +      for (i = 0; i < ARRAY_SIZE (commands); i++) {
> +	 command = &commands[i];
> +
> + -	fprintf (stderr, "\t%s\t%s\n\n", command->name, command->summary);
> + +	fprintf (out, "\t%s\t%s\n\n", command->name, command->summary);
> +      }
> +
> + -    fprintf (stderr, "Use \"notmuch help <command>\" for more details on
> + each command.\n\n");
> + +    fprintf (out, "Use \"notmuch help <command>\" for more details on each
> + command.\n\n");
> +  }
> +
> +  static int
> + @@ -183,8 +183,8 @@ notmuch_help_command (unused (void *ctx), int argc, char
> + *argv[])
> +      unsigned int i;
> +
> +      if (argc == 0) {
> + -	fprintf (stderr, "The notmuch mail system.\n\n");
> + -	usage ();
> + +	fprintf (stdout, "The notmuch mail system.\n\n");
> + +	usage (stdout);
> +	 return 0;
> +      }
> +
> + @@ -192,8 +192,8 @@ notmuch_help_command (unused (void *ctx), int argc, char
> + *argv[])
> +	 command = &commands[i];
> +
> +	 if (strcmp (argv[0], command->name) == 0) {
> + -	    fprintf (stderr, "Help for \"notmuch %s\":\n\n", argv[0]);
> + -	    fprintf (stderr, "\t%s\t%s\n\n%s\n\n", command->name,
> + +	    fprintf (stdout, "Help for \"notmuch %s\":\n\n", argv[0]);
> + +	    fprintf (stdout, "\t%s\t%s\n\n%s\n\n", command->name,
> +		      command->summary, command->documentation);
> +	     return 0;
> +	 }
> + [ application/pgp-signature ]
> + [ text/plain ]
> + [ 4-line signature. Click/Enter to show. ]
> diff --git a/test/emacs-show.expected-output/notmuch-show-buttons-filename b/test/emacs-show.expected-output/notmuch-show-buttons-filename
> new file mode 100644
> index 0000000..8fb4d89
> --- /dev/null
> +++ b/test/emacs-show.expected-output/notmuch-show-buttons-filename
> @@ -0,0 +1,74 @@
> +Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed)
> +Subject: [notmuch] "notmuch help" outputs to stderr?
> +To: notmuch <notmuch@notmuchmail.org>
> +Date: Tue, 17 Nov 2009 19:58:29 -0500
> +
> +[ multipart/mixed ]
> +[ multipart/signed ]
> +[ multipart/mixed ]
> +[ text/plain ]
> +I'm just noticing that 'notmuch help ...' outputs to stderr, which
> +isn't terribly intuitive.  For example, the obvious invocation:
> +
> +  notmuch help | less
> +
> +...isn't terribly helpful.
> +
> +I've attached a patch that lets usage() take a FILE * argument so that
> +you can output to stderr in response to usage errors, and stdout in
> +response to an explicit request.
> +
> +[ 4-line signature. Click/Enter to show. ]
> +[ notmuch-help.patch: text/plain ]
> +diff --git a/notmuch.c b/notmuch.c
> +index c47e640..a35cb99 100644
> +--- a/notmuch.c
> ++++ b/notmuch.c
> +@@ -157,23 +157,23 @@ command_t commands[] = {
> + };
> +
> + static void
> +-usage (void)
> ++usage (FILE *out)
> + {
> +     command_t *command;
> +     unsigned int i;
> +
> +-    fprintf (stderr, "Usage: notmuch <command> [args...]\n");
> +-    fprintf (stderr, "\n");
> +-    fprintf (stderr, "Where <command> and [args...] are as follows:\n");
> +-    fprintf (stderr, "\n");
> ++    fprintf (out, "Usage: notmuch <command> [args...]\n");
> ++    fprintf (out, "\n");
> ++    fprintf (out, "Where <command> and [args...] are as follows:\n");
> ++    fprintf (out, "\n");
> +
> +     for (i = 0; i < ARRAY_SIZE (commands); i++) {
> +	command = &commands[i];
> +
> +-	fprintf (stderr, "\t%s\t%s\n\n", command->name, command->summary);
> ++	fprintf (out, "\t%s\t%s\n\n", command->name, command->summary);
> +     }
> +
> +-    fprintf (stderr, "Use \"notmuch help <command>\" for more details on each
> +command.\n\n");
> ++    fprintf (out, "Use \"notmuch help <command>\" for more details on each
> +command.\n\n");
> + }
> +
> + static int
> +@@ -183,8 +183,8 @@ notmuch_help_command (unused (void *ctx), int argc, char
> +*argv[])
> +     unsigned int i;
> +
> +     if (argc == 0) {
> +-	fprintf (stderr, "The notmuch mail system.\n\n");
> +-	usage ();
> ++	fprintf (stdout, "The notmuch mail system.\n\n");
> ++	usage (stdout);
> +	return 0;
> +     }
> +[ application/pgp-signature ]
> +[ text/plain ]
> +[ 4-line signature. Click/Enter to show. ]
> + Lars Kellogg-Stedman <lars@seas.harvard.edu> (2009-11-18) (attachment inbox signed)
> -- 
> 1.8.5.2.8.g0f6c0d1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[W. Trevor King]

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

On Mon, Feb 03, 2014 at 10:44:39PM +0200, Tomi Ollila wrote:
> having notmuch patch email as expected test output feels a bit
> confusing to me -- especially as 'git grep' may catch some of the
> (possibly future-outdated) content...

There were two very similar patches with filename attachments in the
corpus, so I used them for the tests ;).  I can convert that test to
use emacs_fcc_message or some other auto-generated content if you'd
prefer.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

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

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[W. Trevor King]

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

On Mon, Feb 03, 2014 at 12:34:18PM -0800, W. Trevor King wrote:
> On Mon, Feb 03, 2014 at 08:15:35PM +0000, Mark Walters wrote:
> > I think we should only display/use the content-description if it
> > exists and is non-empty.
> 
> Sounds good to me.  I'll queue this and a test for v2.

Rather than patching this in Emacs, maybe we should collapse the “not
set” and “set to empty string” cases in notmuch-show.c?  I can't think
of any reasons why someone would want to distinguish those two cases,
and it's easier all around if we standardize the representation as far
upstream as possible.

Thoughts?
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

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

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[Tomi Ollila]

On Tue, Feb 04 2014, "W. Trevor King" <wking@tremily.us> wrote:

> On Mon, Feb 03, 2014 at 10:44:39PM +0200, Tomi Ollila wrote:
>> having notmuch patch email as expected test output feels a bit
>> confusing to me -- especially as 'git grep' may catch some of the
>> (possibly future-outdated) content...
>
> There were two very similar patches with filename attachments in the
> corpus, so I used them for the tests ;).  I can convert that test to
> use emacs_fcc_message or some other auto-generated content if you'd
> prefer.

You're right; corpus seems to be full of notmuch patch emails...


... and (although?) (just noticed)...

$ grep '^ *[+-].*\<if\>' test/* test/*/*
test/emacs.expected-output/attachment:+    if (pw_buf_size == -1)
pw_buf_size = 64;
test/emacs.expected-output/attachment:+    if (pw_buf_size == -1)
pw_buf_size = 64;

... I am not giving much resistance to keep the patch as it is :D

>
> Cheers,
> Trevor
>

Tomi

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[David Bremner]

"W. Trevor King" <wking@tremily.us> writes:

>
> Rather than patching this in Emacs, maybe we should collapse the “not
> set” and “set to empty string” cases in notmuch-show.c?  I can't think
> of any reasons why someone would want to distinguish those two cases,
> and it's easier all around if we standardize the representation as far
> upstream as possible.
>

Do the RFCs have anything to say about headers with empty content? If
not I'd be inclined to leave the CLI output as raw as possible, just
because people are always finding new ways to apply tools.

d

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[W. Trevor King]

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

On Sat, Feb 08, 2014 at 08:55:02AM -0400, David Bremner wrote:
> "W. Trevor King" <wking@tremily.us> writes:
> > Rather than patching this in Emacs, maybe we should collapse the
> > “not set” and “set to empty string” cases in notmuch-show.c?  I
> > can't think of any reasons why someone would want to distinguish
> > those two cases, and it's easier all around if we standardize the
> > representation as far upstream as possible.
> 
> Do the RFCs have anything to say about headers with empty content?
> If not I'd be inclined to leave the CLI output as raw as possible,
> just because people are always finding new ways to apply tools.

RFC 2183 does not describe Content-Description, it just uses it in
some examples [1].  In all the examples where Content-Description is
present, the value is not empty.  RFC 2045 defines
Content-Description, but it doesn't give all that much information
[2]:

  The ability to associate some descriptive information with a given
  body is often desirable.  For example, it may be useful to mark an
  "image" body as "a picture of the Space Shuttle Endeavor."  Such
  text may be placed in the Content-Description header field.  This
  header field is always optional.

    description := "Content-Description" ":" *text

  The description is presumed to be given in the US-ASCII character
  set, although the mechanism specified in RFC 2047 may be used for
  non-US-ASCII Content-Description values.

I couldn't find more generic references to the meaning of empty header
values, but I find it hard to imagine anyone assigning semantic value
to an explicitly-empty description (vs. no Content-Description at
all).

Cheers,
Trevor

[1]: http://tools.ietf.org/html/rfc2183#section-3
[2]: http://tools.ietf.org/html/rfc2045#section-8

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

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

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[W. Trevor King]

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

On Mon, Feb 03, 2014 at 12:34:18PM -0800, W. Trevor King wrote:
> On Mon, Feb 03, 2014 at 08:15:35PM +0000, Mark Walters wrote:
> > My only other comment on the series is that you should update
> > devel/schemata in the first patch to reflect this addition.
> 
> Ah, I didn't realize that file existed :p.

Looking at this more, I'm unsure about whether or not I should bump
the version.  b96ba63 (show: indicate length, encoding of omitted body
content, 2012-12-16) added part.content-length? and
part.content-transfer-encoding? and doesn't bump the version, while
abeac48 (search: Add stable queries to thread search results,
2013-10-24) adds thread_summary.query and does bump the version.  From
notmuch-client.h:

 Backwards-incompatible changes such as removing map fields, changing
 the meaning of map fields, or changing the meanings of list elements
 should increase this.  New (required) map fields can be added without
 increasing this.

I think that the addition of part.content-description? should not bump
the version, but then I'm not sure how to document the change in
devel/schemata.  I'm leaning towards something like:

  diff --git a/devel/schemata b/devel/schemata
  index 41dc4a6..63d8aa4 100644
  --- a/devel/schemata
  +++ b/devel/schemata
  @@ -26,6 +26,9 @@ v1
   v2
   - Added the thread_summary.query field.

  +Staged for v3
  +- Added the part.content-descrition? field.
  +
   Common non-terminals
   --------------------
  …

Alternatively, there could be a minor version that gets bumped on each
tweak, however insignificant.

Thoughts?

Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

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

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[Mark Walters]

Initially I agreed with Bremner that we should be as faithful as
possible in our json/sexp output. However, looking at other headers like
cc: it seems that this can be present but empty (at least I sent myself
a message with that property), but that notmuch-show omits it.

Looking at the code for that pathway we use
g_mime_message_get_recipients followed by
internet_address_list_to_string and we only output a cc: pair if this is
non-null (which means we had an address)

In light of that I think changing the cli to only output
content-description if non-null seems consistent.

Best wishes

Mark





On Sat, 08 Feb 2014, "W. Trevor King" <wking@tremily.us> wrote:
> On Sat, Feb 08, 2014 at 08:55:02AM -0400, David Bremner wrote:
>> "W. Trevor King" <wking@tremily.us> writes:
>> > Rather than patching this in Emacs, maybe we should collapse the
>> > “not set” and “set to empty string” cases in notmuch-show.c?  I
>> > can't think of any reasons why someone would want to distinguish
>> > those two cases, and it's easier all around if we standardize the
>> > representation as far upstream as possible.
>> 
>> Do the RFCs have anything to say about headers with empty content?
>> If not I'd be inclined to leave the CLI output as raw as possible,
>> just because people are always finding new ways to apply tools.
>
> RFC 2183 does not describe Content-Description, it just uses it in
> some examples [1].  In all the examples where Content-Description is
> present, the value is not empty.  RFC 2045 defines
> Content-Description, but it doesn't give all that much information
> [2]:
>
>   The ability to associate some descriptive information with a given
>   body is often desirable.  For example, it may be useful to mark an
>   "image" body as "a picture of the Space Shuttle Endeavor."  Such
>   text may be placed in the Content-Description header field.  This
>   header field is always optional.
>
>     description := "Content-Description" ":" *text
>
>   The description is presumed to be given in the US-ASCII character
>   set, although the mechanism specified in RFC 2047 may be used for
>   non-US-ASCII Content-Description values.
>
> I couldn't find more generic references to the meaning of empty header
> values, but I find it hard to imagine anyone assigning semantic value
> to an explicitly-empty description (vs. no Content-Description at
> all).
>
> Cheers,
> Trevor
>
> [1]: http://tools.ietf.org/html/rfc2183#section-3
> [2]: http://tools.ietf.org/html/rfc2045#section-8
>
> -- 
> This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
> For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[Mark Walters]

On Sat, 08 Feb 2014, "W. Trevor King" <wking@tremily.us> wrote:

> On Mon, Feb 03, 2014 at 12:34:18PM -0800, W. Trevor King wrote:
>> On Mon, Feb 03, 2014 at 08:15:35PM +0000, Mark Walters wrote:
>> > My only other comment on the series is that you should update
>> > devel/schemata in the first patch to reflect this addition.
>> 
>> Ah, I didn't realize that file existed :p.
>
> Looking at this more, I'm unsure about whether or not I should bump
> the version.  b96ba63 (show: indicate length, encoding of omitted body
> content, 2012-12-16) added part.content-length? and
> part.content-transfer-encoding? and doesn't bump the version, while
> abeac48 (search: Add stable queries to thread search results,
> 2013-10-24) adds thread_summary.query and does bump the version.  From
> notmuch-client.h:
>
>  Backwards-incompatible changes such as removing map fields, changing
>  the meaning of map fields, or changing the meanings of list elements
>  should increase this.  New (required) map fields can be added without
>  increasing this.
>
> I think that the addition of part.content-description? should not bump
> the version, but then I'm not sure how to document the change in
> devel/schemata.  I'm leaning towards something like:

I think you just add the new field to the list of fields in the schemata
in the appropriate place in the part section as

content-description?: string,

and you don't need to bump the version. I think this is because a client
can safely ask for this field regardless of the notmuch version and
won't get confused. 

Best wishes

Mark




>
>   diff --git a/devel/schemata b/devel/schemata
>   index 41dc4a6..63d8aa4 100644
>   --- a/devel/schemata
>   +++ b/devel/schemata
>   @@ -26,6 +26,9 @@ v1
>    v2
>    - Added the thread_summary.query field.
>
>   +Staged for v3
>   +- Added the part.content-descrition? field.
>   +
>    Common non-terminals
>    --------------------
>   …
>
> Alternatively, there could be a minor version that gets bumped on each
> tweak, however insignificant.
>
> Thoughts?
>
> Trevor
>
> -- 
> This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
> For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

Re: [PATCH 2/2] emacs: Prefer Content-Description over filename for part buttons

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:


> Looking at the code for that pathway we use
> g_mime_message_get_recipients followed by
> internet_address_list_to_string and we only output a cc: pair if this is
> non-null (which means we had an address)
>
> In light of that I think changing the cli to only output
> content-description if non-null seems consistent.
>

OK, I can live with it. Thanks for the analysis Mark.

Cheers,

d