unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] Add --message-headers flag to notmuch-show
@ 2019-11-10 12:49 Johan Parin
  2019-11-10 12:56 ` Johan Parin
  2019-11-11 15:26 ` Daniel Kahn Gillmor
  0 siblings, 2 replies; 14+ messages in thread
From: Johan Parin @ 2019-11-10 12:49 UTC (permalink / raw)
  To: notmuch; +Cc: Johan Parin

Add a new flag --message-headers to notmuch show, in order to let the
user specify displayed headers using `notmuch-message-headers' in the
emacs mua.

The flag will impact which headers are output in
format_headers_sprinter.

By default only the following headers are output by notmuch show with
--format=sexp :

- From
- To
- Subject
- Cc
- Bcc
- Reply-To
- In-reply-to
- References
- Date

`From' is always output regardless of what is specified in
--message-headers.

See this bug report:

  https://notmuchmail.org/pipermail/notmuch/2017/026069.html

This commit does not include documentation updates.
---
 emacs/notmuch-query.el |   4 +-
 emacs/notmuch-tree.el  |   2 +
 notmuch-show.c         | 138 +++++++++++++++++++++++++++++------------
 3 files changed, 104 insertions(+), 40 deletions(-)

diff --git a/emacs/notmuch-query.el b/emacs/notmuch-query.el
index 563e4acf..61c921a4 100644
--- a/emacs/notmuch-query.el
+++ b/emacs/notmuch-query.el
@@ -30,7 +30,9 @@ A thread is a forest or list of trees. A tree is a two element
 list where the first element is a message, and the second element
 is a possibly empty forest of replies.
 "
-  (let ((args '("show" "--format=sexp" "--format-version=4")))
+  (let ((args `("show" "--format=sexp" "--format-version=4"
+		,(concat "--message-headers="
+                         (mapconcat #'identity notmuch-message-headers ",")))))
     (if notmuch-show-process-crypto
 	(setq args (append args '("--decrypt=true"))))
     (setq args (append args search-terms))
diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
index c00315e8..1d793ec3 100644
--- a/emacs/notmuch-tree.el
+++ b/emacs/notmuch-tree.el
@@ -922,6 +922,8 @@ the same as for the function notmuch-tree."
     (let ((proc (notmuch-start-notmuch
 		 "notmuch-tree" (current-buffer) #'notmuch-tree-process-sentinel
 		 "show" "--body=false" "--format=sexp" "--format-version=4"
+                 (concat "--message-headers="
+                         (mapconcat #'identity notmuch-message-headers ","))
 		 message-arg search-args))
 	  ;; Use a scratch buffer to accumulate partial output.
 	  ;; This buffer will be killed by the sentinel, which
diff --git a/notmuch-show.c b/notmuch-show.c
index 21792a57..1658b66e 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -18,11 +18,31 @@
  * Author: Carl Worth <cworth@cworth.org>
  */
 
+#include <string.h>
+#include <stdlib.h>
+
 #include "notmuch-client.h"
 #include "gmime-filter-reply.h"
 #include "sprinter.h"
 #include "zlib-extra.h"
 
+/* Max number of headers that can be output from
+ * format_headers_sprinter */
+#define MAX_PRINTABLE_MESSAGE_HEADERS 25
+
+/* Default list of header names to be printed by
+ * format_headers_sprinter */
+static const char *default_message_header_list[] = {
+    "To", "Subject", "Cc", "Bcc", "Reply-To", "In-reply-to",
+    "References", "Date"};
+
+/* List of header names to be printed by format_headers_sprinter */
+static char **message_header_list_p = (char **) default_message_header_list;
+
+static int message_header_list_len =
+    sizeof(default_message_header_list) / sizeof(char *);
+
+
 static const char *
 _get_tags_as_string (const void *ctx, notmuch_message_t *message)
 {
@@ -48,6 +68,26 @@ _get_tags_as_string (const void *ctx, notmuch_message_t *message)
     return result;
 }
 
+/* Extract requested header names from the message-headers command
+ * line argument.
+ */
+static void
+extract_requested_headers (const char *opt_str)
+{
+    int count = 0;
+    char *tofree = strdup (opt_str);
+    char *string = tofree;
+    char *header_name;
+
+    message_header_list_p = malloc(MAX_PRINTABLE_MESSAGE_HEADERS * sizeof(char *));
+    while ((header_name = strsep(&string, ",")) != NULL &&
+	   count < MAX_PRINTABLE_MESSAGE_HEADERS)
+	message_header_list_p[count++] = strdup(header_name);
+
+    message_header_list_len = count;
+    free(tofree);
+}
+
 /* Get a nice, single-line summary of message. */
 static const char *
 _get_one_line_summary (const void *ctx, notmuch_message_t *message)
@@ -202,57 +242,72 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage *message,
     /* Any changes to the JSON or S-Expression format should be
      * reflected in the file devel/schemata. */
 
-    char *recipients_string;
-    const char *reply_to_string;
     void *local = talloc_new (sp);
+    GMimeHeaderList *header_list;
 
-    sp->begin_map (sp);
+    /* Not currently used */
+    (void) reply;
 
-    sp->map_key (sp, "Subject");
-    if (msg_crypto && msg_crypto->payload_subject) {
-	sp->string (sp, msg_crypto->payload_subject);
-    } else
-	sp->string (sp, g_mime_message_get_subject (message));
+    sp->begin_map (sp);
 
     sp->map_key (sp, "From");
     sp->string (sp, g_mime_message_get_from_string (message));
 
-    recipients_string = g_mime_message_get_address_string (message, GMIME_ADDRESS_TYPE_TO);
-    if (recipients_string) {
-	sp->map_key (sp, "To");
-	sp->string (sp, recipients_string);
-	g_free (recipients_string);
-    }
+    header_list  = g_mime_object_get_header_list (GMIME_OBJECT(message));
 
-    recipients_string = g_mime_message_get_address_string (message, GMIME_ADDRESS_TYPE_CC);
-    if (recipients_string) {
-	sp->map_key (sp, "Cc");
-	sp->string (sp, recipients_string);
-	g_free (recipients_string);
-    }
+    for (int i = 0; i < message_header_list_len; i++) {
+	const char *name = message_header_list_p[i];
 
-    recipients_string = g_mime_message_get_address_string (message, GMIME_ADDRESS_TYPE_BCC);
-    if (recipients_string) {
-	sp->map_key (sp, "Bcc");
-	sp->string (sp, recipients_string);
-	g_free (recipients_string);
-    }
+	if (!STRNCMP_LITERAL (name, "Subject")) {
+	    sp->map_key (sp, "Subject");
+	    if (msg_crypto && msg_crypto->payload_subject) {
+		sp->string (sp, msg_crypto->payload_subject);
+	    } else
+		sp->string (sp, g_mime_message_get_subject (message));
+	}
 
-    reply_to_string = g_mime_message_get_reply_to_string (local, message);
-    if (reply_to_string) {
-	sp->map_key (sp, "Reply-To");
-	sp->string (sp, reply_to_string);
-    }
+	else if (!STRNCMP_LITERAL (name, "To") ||
+		 !STRNCMP_LITERAL (name, "Cc") ||
+		 !STRNCMP_LITERAL (name, "Bcc")) {
+	    GMimeAddressType addr_type;
+	    char *recipients_string;
+
+	    if (!STRNCMP_LITERAL (name, "To"))
+		addr_type = GMIME_ADDRESS_TYPE_TO;
+	    else if (!STRNCMP_LITERAL (name, "Cc"))
+		addr_type = GMIME_ADDRESS_TYPE_CC;
+	    else
+		addr_type = GMIME_ADDRESS_TYPE_BCC;
+
+	    recipients_string = g_mime_message_get_address_string (
+		message, addr_type);
+	    if (recipients_string) {
+		sp->map_key (sp, name);
+		sp->string (sp, recipients_string);
+		g_free (recipients_string);
+	    }
+	}
 
-    if (reply) {
-	sp->map_key (sp, "In-reply-to");
-	sp->string (sp, g_mime_object_get_header (GMIME_OBJECT (message), "In-reply-to"));
+	else if (!STRNCMP_LITERAL (name, "Reply-To")) {
+	    const char *reply_to_string;
 
-	sp->map_key (sp, "References");
-	sp->string (sp, g_mime_object_get_header (GMIME_OBJECT (message), "References"));
-    } else {
-	sp->map_key (sp, "Date");
-	sp->string (sp, g_mime_message_get_date_string (sp, message));
+	    reply_to_string = g_mime_message_get_reply_to_string (local, message);
+	    if (reply_to_string) {
+		sp->map_key (sp, "Reply-To");
+		sp->string (sp, reply_to_string);
+	    }
+	}
+
+	else {
+	    GMimeHeader *header = g_mime_header_list_get_header(
+		header_list, name);
+
+	    if (header == NULL)
+		continue;
+
+	    sp->map_key (sp, g_mime_header_get_name(header));
+	    sp->string (sp, g_mime_header_get_value(header));
+	}
     }
 
     sp->end (sp);
@@ -1168,6 +1223,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
     bool exclude = true;
     bool entire_thread_set = false;
     bool single_message;
+    const char *message_header_str = NULL;
 
     notmuch_opt_desc_t options[] = {
 	{ .opt_keyword = &format, .name = "format", .keywords =
@@ -1193,6 +1249,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	{ .opt_bool = &params.output_body, .name = "body" },
 	{ .opt_bool = &params.include_html, .name = "include-html" },
 	{ .opt_inherit = notmuch_shared_options },
+	{ .opt_string = &message_header_str, .name = "message-headers" },
 	{ }
     };
 
@@ -1202,6 +1259,9 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 
     notmuch_process_shared_options (argv[0]);
 
+    if (message_header_str)
+	extract_requested_headers(message_header_str);
+
     /* explicit decryption implies verification */
     if (params.crypto.decrypt == NOTMUCH_DECRYPT_NOSTASH ||
 	params.crypto.decrypt == NOTMUCH_DECRYPT_TRUE)
-- 
2.21.0 (Apple Git-122)

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-10 12:49 [PATCH] Add --message-headers flag to notmuch-show Johan Parin
@ 2019-11-10 12:56 ` Johan Parin
  2019-11-11 15:26 ` Daniel Kahn Gillmor
  1 sibling, 0 replies; 14+ messages in thread
From: Johan Parin @ 2019-11-10 12:56 UTC (permalink / raw)
  To: notmuch


According to the proposal of Jani and Tomi I have made a new patch
version which adds a flag --message-headers to notmuch show. This is a
preliminary commit just to get general comments.

No doc updates here. Also there are probably a few style issues. And I
guess I should use talloc instead of malloc.

/Johan

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-10 12:49 [PATCH] Add --message-headers flag to notmuch-show Johan Parin
  2019-11-10 12:56 ` Johan Parin
@ 2019-11-11 15:26 ` Daniel Kahn Gillmor
  2019-11-11 15:39   ` Daniel Kahn Gillmor
  1 sibling, 1 reply; 14+ messages in thread
From: Daniel Kahn Gillmor @ 2019-11-11 15:26 UTC (permalink / raw)
  To: Johan Parin, notmuch

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

Hi Johan--

On Sun 2019-11-10 13:49:29 +0100, Johan Parin wrote:
> Add a new flag --message-headers to notmuch show, in order to let the
> user specify displayed headers using `notmuch-message-headers' in the
> emacs mua.

This is interesting work, thanks for proposing it.

I haven't reviewed the C changes in detail, but i wanted to ask a couple
of bigger-picture questions about where you see this going and how it
fits into the broader ecosystem around notmuch:

 - What is the specific use case for this? For example, can you identify
   situations where different headers need to be emitted by different
   users?  Even one motivating example would help others on this list
   understand why they might want to care :)

 - Do we need full configurability here?  I'd generally prefer for
   notmuch to be simple, instead of offering lots of ways for things to
   be subtly different across installations.  If there's an additional
   header that notmuch-show should be exporting in machine-readable
   mode, why not just export it unilaterally, and let the consumer of
   the headers filter out what they want to filter out?

 - If we do go ahead with the configurability approach, is there a
   rationale for requiring that the option should be a full list, rather
   than a differential approach?  for example "--include-header=Foo" and
   "--suppress-header=Bar" would let the user stick as close to the
   defaults as possible.  That way an upgrade to notmuch that does
   something nice to the default headers wouldn't necessarily get
   overridden by anyone in the habit of making these adjustments.

 - Again, if we're going with the configurability approach, should it
   just be a command line argument, or is this something that someone
   might want to set/retrieve with "notmuch config"?

These are meant as constructive questions, not as a critique -- i'm
hoping that we can make notmuch solve the problems you're trying to
solve!

All the best,

        --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-11 15:26 ` Daniel Kahn Gillmor
@ 2019-11-11 15:39   ` Daniel Kahn Gillmor
  2019-11-12 15:48     ` Antoine Beaupré
  2019-11-12 19:24     ` Johan Parin
  0 siblings, 2 replies; 14+ messages in thread
From: Daniel Kahn Gillmor @ 2019-11-11 15:39 UTC (permalink / raw)
  To: Johan Parin, notmuch

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

On Mon 2019-11-11 10:26:18 -0500, Daniel Kahn Gillmor wrote:
>  - What is the specific use case for this? For example, can you identify
>    situations where different headers need to be emitted by different
>    users?  Even one motivating example would help others on this list
>    understand why they might want to care :)

ah, sorry, i've just read id:20191109221358.4349-1-johan.parin@gmail.com
and its associated messages, so i can see that some of the questions i'm
asking are already under discussion.

I see that you just want user-agent and x-mailer for your own purposes.

Maybe it would be worthwhile to propose that narrow, limited change as a
simple patch, without configurability and see what it looks like?  I
would personally be more likely to advocate for merging a patch that
meets the specific needs of a notmuch user, and increase the
configurability surface of notmuch.

If processing a couple of extra headers on a long thread is too
expensive for some consumer, i'd suggest that is an optimization for the
consumer to tackle.

    --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-11 15:39   ` Daniel Kahn Gillmor
@ 2019-11-12 15:48     ` Antoine Beaupré
  2019-11-12 17:19       ` Daniel Kahn Gillmor
  2019-11-12 19:24     ` Johan Parin
  1 sibling, 1 reply; 14+ messages in thread
From: Antoine Beaupré @ 2019-11-12 15:48 UTC (permalink / raw)
  To: notmuch

On 2019-11-11 10:39:11, Daniel Kahn Gillmor wrote:
> On Mon 2019-11-11 10:26:18 -0500, Daniel Kahn Gillmor wrote:
>>  - What is the specific use case for this? For example, can you identify
>>    situations where different headers need to be emitted by different
>>    users?  Even one motivating example would help others on this list
>>    understand why they might want to care :)
>
> ah, sorry, i've just read id:20191109221358.4349-1-johan.parin@gmail.com
> and its associated messages, so i can see that some of the questions i'm
> asking are already under discussion.
>
> I see that you just want user-agent and x-mailer for your own purposes.
>
> Maybe it would be worthwhile to propose that narrow, limited change as a
> simple patch, without configurability and see what it looks like?  I
> would personally be more likely to advocate for merging a patch that
> meets the specific needs of a notmuch user, and increase the
> configurability surface of notmuch.
>
> If processing a couple of extra headers on a long thread is too
> expensive for some consumer, i'd suggest that is an optimization for the
> consumer to tackle.

They need User-Agent:, I want Archive-At:, they will want
X-Mailer... when is it going to stop? I would rather have
configurability here than and endless stream of patches to grow a
possibly boundless list...

a.

-- 
Brief is this existence, as a fleeting visit in a strange house.
The path to be pursued is poorly lit by a flickering consciousness.
                       - Albert Einstein

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-12 15:48     ` Antoine Beaupré
@ 2019-11-12 17:19       ` Daniel Kahn Gillmor
  2019-11-12 17:27         ` Antoine Beaupré
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Kahn Gillmor @ 2019-11-12 17:19 UTC (permalink / raw)
  To: Antoine Beaupré, notmuch

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

On Tue 2019-11-12 10:48:54 -0500, Antoine Beaupré wrote:
> They need User-Agent:, I want Archive-At:, they will want
> X-Mailer... when is it going to stop?

It's going to stop when users are satisfied. :)  I highly doubt that we
will reach all possible headers.

> I would rather have configurability here than and endless stream of
> patches to grow a possibly boundless list...

Configurability is at least as expensive to maintain as a (not actually
endless) short stream of patches, but i understand different people have
different tradeoffs they're willing to make.

Can you suggest which kind of configurability you would prefer: complete
override, or differential modification?  And if so, why do you prefer it
over the other choice?  or should there be both?

          --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-12 17:19       ` Daniel Kahn Gillmor
@ 2019-11-12 17:27         ` Antoine Beaupré
  0 siblings, 0 replies; 14+ messages in thread
From: Antoine Beaupré @ 2019-11-12 17:27 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, notmuch

On 2019-11-12 12:19:01, Daniel Kahn Gillmor wrote:
> On Tue 2019-11-12 10:48:54 -0500, Antoine Beaupré wrote:
>> They need User-Agent:, I want Archive-At:, they will want
>> X-Mailer... when is it going to stop?
>
> It's going to stop when users are satisfied. :)  I highly doubt that we
> will reach all possible headers.

I wouldn't be surprised, actually. :p

>> I would rather have configurability here than and endless stream of
>> patches to grow a possibly boundless list...
>
> Configurability is at least as expensive to maintain as a (not actually
> endless) short stream of patches, but i understand different people have
> different tradeoffs they're willing to make.
>
> Can you suggest which kind of configurability you would prefer: complete
> override, or differential modification?  And if so, why do you prefer it
> over the other choice?  or should there be both?

I have no idea, to be honest. But it's something I banged my head a few
times against in notmuch and I would hate to have to recompile the
entire thing to fix this on a local copy.

a.

-- 
The idea that Bill Gates has appeared like a knight in shining armour to
lead all customers out of a mire of technological chaos neatly ignores
the fact that it was he who, by peddling second-rate technology, led
them into it in the first place. - Douglas Adams (1952-2001)

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-11 15:39   ` Daniel Kahn Gillmor
  2019-11-12 15:48     ` Antoine Beaupré
@ 2019-11-12 19:24     ` Johan Parin
  2019-11-12 22:19       ` Daniel Kahn Gillmor
  1 sibling, 1 reply; 14+ messages in thread
From: Johan Parin @ 2019-11-12 19:24 UTC (permalink / raw)
  To: notmuch


Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> On Mon 2019-11-11 10:26:18 -0500, Daniel Kahn Gillmor wrote:
>>  - What is the specific use case for this? For example, can you identify
>>    situations where different headers need to be emitted by different
>>    users?  Even one motivating example would help others on this list
>>    understand why they might want to care :)
>
> ah, sorry, i've just read
> id:20191109221358.4349-1-johan.parin@gmail.com and its associated
> messages, so i can see that some of the questions i'm asking are
> already under discussion.
>
> I see that you just want user-agent and x-mailer for your own
> purposes.
>

Well that is just my current minimum requirements. I'm a mail nerd and I
always want to see what mua people are using, if present. And no, it is
not OK to have to hit 'V' and see all headers. There are way too many
garbage headers. You want to have the headers you are interested in to
be displayed automatically for every message in a nice readable fashion.

Someone else in this thread wanted Arhive-At; the bug reporter wanted
X-Github-Sender. Now that I think about it I definitely want
X-Face. Maybe List-Id. etc. It's really hard to predict what people
think is interesting, which is why configurability is needed.

I think this is quite basic functionality. All "nerdy" muas have it ;)
Gnus has it, mu4e has it etc. mutt seems to have some reasonable default
of at least displaying User-Agent. I think Thunderbird has it too. etc

> Maybe it would be worthwhile to propose that narrow, limited change as
> a simple patch, without configurability and see what it looks like?  I
> would personally be more likely to advocate for merging a patch that
> meets the specific needs of a notmuch user, and increase the
> configurability surface of notmuch.
>

I really don't think the configurability is expensive. The elisp code
base is already prepared for it with the notmuch-message-headers
variable. It's just that it's not working because notmuch-show.c only
exports a fixed set of headers. So it's a bug fix, really.

> If processing a couple of extra headers on a long thread is too
> expensive for some consumer, i'd suggest that is an optimization for
> the consumer to tackle.

I have implemented three variants:

a) notmuch-show.c exports all headers
b) notmuch-show.c exports a limited set of "interesting" headers. This
   patch should be modified to set the headers in the config file. I
   think this is a really good solution if you are concerned with
   performance in a)
c) notmuch-show.c exports headers specified with a command line arg,
   which is passed from the elisp based on `notmuch-message-headers'.

In all cases the actual headers displayed in the emacs mua is controlled
by `notmuch-message-headers'.

Personally I don't think there is any discernible performance impact
with any of these, and that any performance impact is dwarfed by the
GMime message parsing code and elisp display code. But if there is
concern with a, then b) and c) can be chosen which should have basically
zero performance impact.

I also don't think there is maintenance cost with any of these once it
is implemented so I don't see why we should not have configurability.

All of the patches need some polishing (and I noticed a functional issue
with "c). But if we can get consensus on an approach that is OK, then I
would be happy to continue perfecting it.

/Johan

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-12 19:24     ` Johan Parin
@ 2019-11-12 22:19       ` Daniel Kahn Gillmor
  2019-11-12 23:30         ` Tomi Ollila
  2019-11-13  9:37         ` David Edmondson
  0 siblings, 2 replies; 14+ messages in thread
From: Daniel Kahn Gillmor @ 2019-11-12 22:19 UTC (permalink / raw)
  To: Johan Parin, notmuch

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

On Tue 2019-11-12 20:24:12 +0100, Johan Parin wrote:
> I think this is quite basic functionality. All "nerdy" muas have it ;)
> Gnus has it, mu4e has it etc. mutt seems to have some reasonable default
> of at least displaying User-Agent. I think Thunderbird has it too. etc

Yes, and of course i accept that notmuch is a "nerdy" MUA.

> I really don't think the configurability is expensive.

To be clear, i'm suggesting that the code maintenance for the
configurability (and the API surface it creates) is what's expensive,
not that the code is particularly costly to run.

> b) notmuch-show.c exports a limited set of "interesting" headers. This
>    patch should be modified to set the headers in the config file. I
>    think this is a really good solution if you are concerned with
>    performance in a)

Are you sure you want the configuration in the config file and not in
the database itself?  we've had a bunch of discussion on-list about the
fact that the config data is split between the config file and the
database, and some of the config must live in the database (because the
config affects library operations, which do not read the config file).

Eventually, i hope we will consolidate on config-in-the-database, so
each additional piece of config we add to the configfile makes me sad.

furthermore, if we ever decide to remove these configuration options
(e.g. by switching to (a)), we'll end up making some users sad.  so it's
ongoing code maintenance even if most people don't use it :(

And, I still haven't heard any clear arguments for choosing between
configurability as an absolute thing or a differential thing.  They have
significantly different impact on adopters over time, as the default
configuration changes.

So, of the three options you list, i far prefer (a) because it doesn't
introduce any of the configurability maintenance or API complexity
costs.

If the main objection to (a) is performance regression, i'd like to see
some profiling of that performance cost, so we can better understand it.
Perhaps there's a different way to mitigate a performance regression.

          --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-12 22:19       ` Daniel Kahn Gillmor
@ 2019-11-12 23:30         ` Tomi Ollila
  2019-11-14  0:23           ` Daniel Kahn Gillmor
  2019-11-13  9:37         ` David Edmondson
  1 sibling, 1 reply; 14+ messages in thread
From: Tomi Ollila @ 2019-11-12 23:30 UTC (permalink / raw)
  To: notmuch

On Tue, Nov 12 2019, Daniel Kahn Gillmor wrote:

>
> And, I still haven't heard any clear arguments for choosing between
> configurability as an absolute thing or a differential thing.  They have
> significantly different impact on adopters over time, as the default
> configuration changes.

I don't understand your question, but I think that configuration option
for choosing what message headers to return is far worst of the options,
mostly because configuration and what frontend may desire goes easily
out if sync (and when managed manually that is what inevitably will
happen). 

> So, of the three options you list, i far prefer (a) because it doesn't
> introduce any of the configurability maintenance or API complexity
> costs.

> If the main objection to (a) is performance regression, i'd like to see
> some profiling of that performance cost, so we can better understand it.
> Perhaps there's a different way to mitigate a performance regression.

I'd guess it depends how frontends spend time parsing json/sexp output. 
I would not expect raw C code to be bottleneck, don't know how gmime
spends time fetching header data on user request...

The option (b) is kinda my favorite, code could be pretty simple, ordering
(sprinted in order listed), duplicates (rescan request list so far and drop
if header found) might be the harders questions (and seemed not ;/). 

If option (b) were taken, status quo -- no change in returned headers
should be maintained -- separate patch series w/ devel/schemata and test
changes can be sent is there desire for changing that.


>
>           --dkg

Tomi

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-12 22:19       ` Daniel Kahn Gillmor
  2019-11-12 23:30         ` Tomi Ollila
@ 2019-11-13  9:37         ` David Edmondson
  1 sibling, 0 replies; 14+ messages in thread
From: David Edmondson @ 2019-11-13  9:37 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, Johan Parin, notmuch

On Tuesday, 2019-11-12 at 17:19:13 -05, Daniel Kahn Gillmor wrote:

> Are you sure you want the configuration in the config file and not in
> the database itself?

That's fine with me.

dme.
-- 
So tap at my window, maybe I might let you in.

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-12 23:30         ` Tomi Ollila
@ 2019-11-14  0:23           ` Daniel Kahn Gillmor
  2019-11-14 20:44             ` Tomi Ollila
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Kahn Gillmor @ 2019-11-14  0:23 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

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

On Wed 2019-11-13 01:30:50 +0200, Tomi Ollila wrote:
> On Tue, Nov 12 2019, Daniel Kahn Gillmor wrote:
>
>> And, I still haven't heard any clear arguments for choosing between
>> configurability as an absolute thing or a differential thing.  They have
>> significantly different impact on adopters over time, as the default
>> configuration changes.
>
> I don't understand your question,

configurability as an absolute thing:

 --message-headers=foo,bar,baz

configurability as a differential thing:

  --add-message-header=foo --suppress-message-header=qux

> but I think that configuration option
> for choosing what message headers to return is far worst of the options,
> mostly because configuration and what frontend may desire goes easily
> out if sync (and when managed manually that is what inevitably will
> happen). 

totally agreed, but this is very different from what you said next:

> The option (b) is kinda my favorite, code could be pretty simple, ordering
> (sprinted in order listed), duplicates (rescan request list so far and drop
> if header found) might be the harders questions (and seemed not ;/). 
>
> If option (b) were taken, status quo -- no change in returned headers
> should be maintained -- separate patch series w/ devel/schemata and test
> changes can be sent is there desire for changing that.

afaict, option (b) seems to contemplate configurability, which you say
above you don't want.  Maybe we need a clearer list of options, because
this is getting confusing :P

      --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-14  0:23           ` Daniel Kahn Gillmor
@ 2019-11-14 20:44             ` Tomi Ollila
  2019-11-15 16:59               ` Daniel Kahn Gillmor
  0 siblings, 1 reply; 14+ messages in thread
From: Tomi Ollila @ 2019-11-14 20:44 UTC (permalink / raw)
  To: notmuch

On Thu, Nov 14 2019, Daniel Kahn Gillmor wrote:

> On Wed 2019-11-13 01:30:50 +0200, Tomi Ollila wrote:
>> On Tue, Nov 12 2019, Daniel Kahn Gillmor wrote:
>>
>>> And, I still haven't heard any clear arguments for choosing between
>>> configurability as an absolute thing or a differential thing.  They have
>>> significantly different impact on adopters over time, as the default
>>> configuration changes.
>>
>> I don't understand your question,
>
> configurability as an absolute thing:
>
>  --message-headers=foo,bar,baz
>
> configurability as a differential thing:
>
>   --add-message-header=foo --suppress-message-header=qux

Ahh...

Osmo A. Wiio was a smart man when he stated: "Communication usually fails,
except by accident" ( http://jkorpela.fi/wiio.html - read it now, I'll wait ).

I understand 'configurability' a bit different way -- store something
somewhere which is fetched in the future and alters behaviuor in that
way.

In above options, --message-headers and so on, I'd just "state" or "demand"
the program in question to give me this information, and don't think there
was any configurability in question ('configuration', in "broader" sense,
is there, just hiding somewhere in background =D)

>
>> but I think that configuration option
>> for choosing what message headers to return is far worst of the options,
>> mostly because configuration and what frontend may desire goes easily
>> out if sync (and when managed manually that is what inevitably will
>> happen). 
>
> totally agreed, but this is very different from what you said next:
>
>> The option (b) is kinda my favorite, code could be pretty simple, ordering
>> (sprinted in order listed), duplicates (rescan request list so far and drop
>> if header found) might be the harders questions (and seemed not ;/). 
>>
>> If option (b) were taken, status quo -- no change in returned headers
>> should be maintained -- separate patch series w/ devel/schemata and test
>> changes can be sent is there desire for changing that.
>
> afaict, option (b) seems to contemplate configurability, which you say
> above you don't want.  Maybe we need a clearer list of options, because
> this is getting confusing :P

Perhaps my explanation above cleared at least a bit of confusion.

W/ all this information, somewhat exhaustive (not by options, but by
resources I put making it) list of thougts.


1a by default behave as it is behaving now

1b alternative, in json and sexp, include *all* headers for the use of
   frontends (in many other email systems frontends parse full email
   messages and see all headers, in notmuch case frontends don't have
   to do so since notmuch did the parsing and provides structured data
   of (currently subset) that information


2a have option --message-headers= -- when used just those headers requested
   is returned (I'd personally prefer this over the "differential" options,
   frontends get exactly what it wants and does not need to consider any
   default where to add of suppress from)

2b have --add-message-header=foo --suppress-message-header=qux -- to modify
   the defult list...

2c have named stored configurations, which can be retrieved with yet another
   command line option, since naming is hard, quick potenttially dumb
   example could be like: --custom-message-headers=my-cli-headers-set-3

I personally would first go with 1a and 2a. 2c sounds interesting but
requires more work (and I could personally use wrapper instead =D)

Anyway, if someone(tm) implements any of these, speed is not a problem, and
code is reasonably maintainable happy to see notmuch improved this way...

>
>       --dkg

Tomi

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

* Re: [PATCH] Add --message-headers flag to notmuch-show
  2019-11-14 20:44             ` Tomi Ollila
@ 2019-11-15 16:59               ` Daniel Kahn Gillmor
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Kahn Gillmor @ 2019-11-15 16:59 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

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

On Thu 2019-11-14 22:44:32 +0200, Tomi Ollila wrote:
> W/ all this information, somewhat exhaustive (not by options, but by
> resources I put making it) list of thougts.
>
>
> 1a by default behave as it is behaving now
>
> 1b alternative, in json and sexp, include *all* headers for the use of
>    frontends (in many other email systems frontends parse full email
>    messages and see all headers, in notmuch case frontends don't have
>    to do so since notmuch did the parsing and provides structured data
>    of (currently subset) that information
>
>
> 2a have option --message-headers= -- when used just those headers requested
>    is returned (I'd personally prefer this over the "differential" options,
>    frontends get exactly what it wants and does not need to consider any
>    default where to add of suppress from)
>
> 2b have --add-message-header=foo --suppress-message-header=qux -- to modify
>    the defult list...
>
> 2c have named stored configurations, which can be retrieved with yet another
>    command line option, since naming is hard, quick potenttially dumb
>    example could be like: --custom-message-headers=my-cli-headers-set-3

thanks for this list.  It seems to miss the other thing that is
currently being contemplated, which is maybe:

  3  have a new "notmuch show" configuration option that affects what
     headers should be emitted

This is distinct from the command-line options detailed in 2a and 2b,
and distinct from the idea of named bundled configuration options in the
config file.

       --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

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

end of thread, other threads:[~2019-11-15 19:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-10 12:49 [PATCH] Add --message-headers flag to notmuch-show Johan Parin
2019-11-10 12:56 ` Johan Parin
2019-11-11 15:26 ` Daniel Kahn Gillmor
2019-11-11 15:39   ` Daniel Kahn Gillmor
2019-11-12 15:48     ` Antoine Beaupré
2019-11-12 17:19       ` Daniel Kahn Gillmor
2019-11-12 17:27         ` Antoine Beaupré
2019-11-12 19:24     ` Johan Parin
2019-11-12 22:19       ` Daniel Kahn Gillmor
2019-11-12 23:30         ` Tomi Ollila
2019-11-14  0:23           ` Daniel Kahn Gillmor
2019-11-14 20:44             ` Tomi Ollila
2019-11-15 16:59               ` Daniel Kahn Gillmor
2019-11-13  9:37         ` David Edmondson

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