Re: [PATCH] Fix format_headers_sprinter to return all headers (v2)
On Sun, Nov 10 2019, David Edmondson wrote: > On Saturday, 2019-11-09 at 23:13:58 +01, Johan Parin wrote: > >> +const char* interesting_headers[] = { + "Maildir", >> "Mailing-list", "Tags", "Attachments", "Signature", + >> "Decryption", "User-agent", "X-Mailer"}; > > It would be convenient to specify these in ~/.notmuch-config > rather than hard-coding them (or specifying them as command line > arguments). If we take the command line option approach, then the frontend can request the headers, and have it configured in one location (setting notmuch-message-headers in emacs affects how emacs mua calls notmuch-show)... Also, different frontends may have different usable headers, for example `X-Face:` probably only works w/ emacs mua =D Tomi > > dme. > -- > And you can't hold me down, 'cause I belong to the hurricane. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Fix format_headers_sprinter to return all headers (v2)
David Edmondson writes: > On Saturday, 2019-11-09 at 23:13:58 +01, Johan Parin wrote: > >> +const char* interesting_headers[] = { + "Maildir", >> "Mailing-list", "Tags", "Attachments", "Signature", + "Decryption", >> "User-agent", "X-Mailer"}; > > It would be convenient to specify these in ~/.notmuch-config rather > than hard-coding them (or specifying them as command line arguments). > I agree that is a better solution, maybe even better than using command line args. I can look into this as well. Will not have much time the next few days though. /Johan ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Fix format_headers_sprinter to return all headers (v2)
On Saturday, 2019-11-09 at 23:13:58 +01, Johan Parin wrote: +const char* interesting_headers[] = { + "Maildir", "Mailing-list", "Tags", "Attachments", "Signature", + "Decryption", "User-agent", "X-Mailer"}; It would be convenient to specify these in ~/.notmuch-config rather than hard-coding them (or specifying them as command line arguments). dme. -- And you can't hold me down, 'cause I belong to the hurricane. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Fix format_headers_sprinter to return all headers
On Sat, 09 Nov 2019, David Bremner wrote: > Jani Nikula writes: > >> On Sat, 09 Nov 2019, Johan Parin wrote: >>> Modify format_headers_sprinter so that it returns all headers in the >>> sexp, instead of a fixed set of headers. >> >> I have to deal with plenty of long threads that already take a very long >> time to open in notmuch-emacs. How's this going to impact the emacs >> interface performance? For example the patch mail I'm replying to has >> more header content than body content. There are tons of headers that I >> can't imagine being useful to anyone. > > Can we benchmark this somehow? I think we have suitable threads in the > LKML corpus, but the challenge is timing emacs in some sensible way. notmuch-emacs performance tests? :o I can of course try this stuff to get a gut feeling, but it's subjective and doesn't really scale. :) BR, Jani. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Fix format_headers_sprinter to return all headers
On Sat, 09 Nov 2019, Johan Parin wrote: > Jani Nikula writes: > >> On Sat, 09 Nov 2019, Johan Parin wrote: >>> Modify format_headers_sprinter so that it returns all headers in the >>> sexp, instead of a fixed set of headers. >> >> I have to deal with plenty of long threads that already take a very >> long time to open in notmuch-emacs. How's this going to impact the >> emacs interface performance? For example the patch mail I'm replying >> to has more header content than body content. There are tons of >> headers that I can't imagine being useful to anyone. >> > > How long are these threads may I ask? Would it be possible for you to > apply the patch and see if you see a noticable difference? I'll need to try this with my work email which is the bigger problem. > I have a hard time seeing that the C code would cause any noticable > performance impact, but perhaps the elisp. But my general understanding > of the code base is low, so I may be wrong. It's indeed the lisp part that I'm worried about. > I guess it's in the following type of code the performance concern lies? > > (mapc (lambda (header) > (let* ((header-symbol (intern (concat ":" header))) > (header-value (plist-get headers header-symbol))) > (if (and header-value > (not (string-equal "" header-value))) > (notmuch-show-insert-header header header-value > notmuch-message-headers) > > Don't know how efficient plists are in general. > >> I'm wondering if the right thing to do would be to make it possible to >> specify which additional headers to include in notmuch-show >> output. This would be based on notmuch-message-headers. We already >> have options to include/exclude body content and html parts for much >> the same reasons. (Though the command-line arguments for those are >> incoherent at best.) >> > > This would be possible of course. > > Another option I guess is to hard code some extra "interesting" > headers. But what is interesting is subjective and it is very nice to > have the flexibility to control this is in elisp. > > mu4e has the customization variable `mu4e-view-fields', but it can only > take on values from a restricted list, namely: > > :from :to :cc :subject :flags :date :maildir :mailing-list :tags > :attachments :signature :decryption :user-agent > > So a compromise would be to restrict the returned headers to a list like > this. This of course is if we think the potential performance issue is > in the elisp. > > Personally I would be content for the moment to be able to see > user-agent and x-mailer. But that's just me. I failed to mention that I do think the feature itself would be useful and welcome. Currently I just hit V and find the header in the raw message... BR, Jani. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Fix format_headers_sprinter to return all headers (v2)
Subject line not updated On Sat, Nov 09 2019, Johan Parin wrote: > So this version only returns a fixed limited set of extra > headers. Hopefully this eliminates any concern for a performance > penalty. Of course it limits the usefulness of the > notmuch-message-headers variable. headers to request based on command line parameter (and default to current set) could be an option (kinda Jani already suggested). Quick (not perhaps well thought) one could be: --message-headers=To,Cc,X-Face and in the other mail -- mostly style comments (for future reference) On Sat, Nov 09 2019, Johan Parin wrote: > Modify format_headers_sprinter so that it returns some additional headers in > the > sexp, instead of a small fixed set of headers. > > This version includes the following headers: > > - Maildir > - Mailing-list > - Tags > - Attachments > - Signature > - Decryption > - User-agent > - X-Mailer > > This is required in order for the elisp variable > `notmuch-message-headers' to work to some extent. > > See this bug report: > > https://notmuchmail.org/pipermail/notmuch/2017/026069.html > --- > notmuch-show.c | 19 +++ > 1 file changed, 19 insertions(+) > > diff --git a/notmuch-show.c b/notmuch-show.c > index 21792a57..86ddb491 100644 > --- a/notmuch-show.c > +++ b/notmuch-show.c > @@ -205,6 +205,12 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage > *message, > char *recipients_string; > const char *reply_to_string; > void *local = talloc_new (sp); > +GMimeHeaderList *header_list; > +GMimeHeader *header; could be dropped here and had only in scope where it is used below... > +const char* interesting_headers[] = { > + "Maildir", "Mailing-list", "Tags", "Attachments", "Signature", > + "Decryption", "User-agent", "X-Mailer"}; > +const int interesting_header_count = sizeof(interesting_headers) / > sizeof(char *); more "idiomatic" could be ... sizeof (interesting_headers) / sizeof (interesting_headers[0]); > sp->begin_map (sp); > > @@ -255,6 +261,19 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage > *message, > sp->string (sp, g_mime_message_get_date_string (sp, message)); > } > > +header_list = g_mime_object_get_header_list (GMIME_OBJECT(message)); two (2) spaces, then no space after GMIME_OBJECT > + > +for (int i = 0; i < interesting_header_count; i++) { > + ... here GMimeHeader *header; is only used > + header = g_mime_header_list_get_header( > + header_list, interesting_headers[i]); > + if (header == NULL) > + continue; > + > + sp->map_key (sp, g_mime_header_get_name(header)); > + sp->string (sp, g_mime_header_get_value(header)); spaces before '(' > +} > + > sp->end (sp); > talloc_free (local); > } > -- > 2.21.0 (Apple Git-122) > > ___ > notmuch mailing list > notmuch@notmuchmail.org > https://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Fix format_headers_sprinter to return all headers (v2)
So this version only returns a fixed limited set of extra headers. Hopefully this eliminates any concern for a performance penalty. Of course it limits the usefulness of the notmuch-message-headers variable. /Johan ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Fix format_headers_sprinter to return all headers (v2)
Modify format_headers_sprinter so that it returns some additional headers in the sexp, instead of a small fixed set of headers. This version includes the following headers: - Maildir - Mailing-list - Tags - Attachments - Signature - Decryption - User-agent - X-Mailer This is required in order for the elisp variable `notmuch-message-headers' to work to some extent. See this bug report: https://notmuchmail.org/pipermail/notmuch/2017/026069.html --- notmuch-show.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/notmuch-show.c b/notmuch-show.c index 21792a57..86ddb491 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -205,6 +205,12 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage *message, char *recipients_string; const char *reply_to_string; void *local = talloc_new (sp); +GMimeHeaderList *header_list; +GMimeHeader *header; +const char* interesting_headers[] = { + "Maildir", "Mailing-list", "Tags", "Attachments", "Signature", + "Decryption", "User-agent", "X-Mailer"}; +const int interesting_header_count = sizeof(interesting_headers) / sizeof(char *); sp->begin_map (sp); @@ -255,6 +261,19 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage *message, sp->string (sp, g_mime_message_get_date_string (sp, message)); } +header_list = g_mime_object_get_header_list (GMIME_OBJECT(message)); + +for (int i = 0; i < interesting_header_count; i++) { + + header = g_mime_header_list_get_header( + header_list, interesting_headers[i]); + 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); talloc_free (local); } -- 2.21.0 (Apple Git-122) ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Fix format_headers_sprinter to return all headers
Jani Nikula writes: > On Sat, 09 Nov 2019, Johan Parin wrote: >> Modify format_headers_sprinter so that it returns all headers in the >> sexp, instead of a fixed set of headers. > > I have to deal with plenty of long threads that already take a very > long time to open in notmuch-emacs. How's this going to impact the > emacs interface performance? For example the patch mail I'm replying > to has more header content than body content. There are tons of > headers that I can't imagine being useful to anyone. > How long are these threads may I ask? Would it be possible for you to apply the patch and see if you see a noticable difference? I have a hard time seeing that the C code would cause any noticable performance impact, but perhaps the elisp. But my general understanding of the code base is low, so I may be wrong. I guess it's in the following type of code the performance concern lies? (mapc (lambda (header) (let* ((header-symbol (intern (concat ":" header))) (header-value (plist-get headers header-symbol))) (if (and header-value (not (string-equal "" header-value))) (notmuch-show-insert-header header header-value notmuch-message-headers) Don't know how efficient plists are in general. > I'm wondering if the right thing to do would be to make it possible to > specify which additional headers to include in notmuch-show > output. This would be based on notmuch-message-headers. We already > have options to include/exclude body content and html parts for much > the same reasons. (Though the command-line arguments for those are > incoherent at best.) > This would be possible of course. Another option I guess is to hard code some extra "interesting" headers. But what is interesting is subjective and it is very nice to have the flexibility to control this is in elisp. mu4e has the customization variable `mu4e-view-fields', but it can only take on values from a restricted list, namely: :from :to :cc :subject :flags :date :maildir :mailing-list :tags :attachments :signature :decryption :user-agent So a compromise would be to restrict the returned headers to a list like this. This of course is if we think the potential performance issue is in the elisp. Personally I would be content for the moment to be able to see user-agent and x-mailer. But that's just me. /Johan ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Fix format_headers_sprinter to return all headers
Jani Nikula writes: > On Sat, 09 Nov 2019, Johan Parin wrote: >> Modify format_headers_sprinter so that it returns all headers in the >> sexp, instead of a fixed set of headers. > > I have to deal with plenty of long threads that already take a very long > time to open in notmuch-emacs. How's this going to impact the emacs > interface performance? For example the patch mail I'm replying to has > more header content than body content. There are tons of headers that I > can't imagine being useful to anyone. Can we benchmark this somehow? I think we have suitable threads in the LKML corpus, but the challenge is timing emacs in some sensible way. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Fix format_headers_sprinter to return all headers
On Sat, 09 Nov 2019, Johan Parin wrote: > Modify format_headers_sprinter so that it returns all headers in the > sexp, instead of a fixed set of headers. I have to deal with plenty of long threads that already take a very long time to open in notmuch-emacs. How's this going to impact the emacs interface performance? For example the patch mail I'm replying to has more header content than body content. There are tons of headers that I can't imagine being useful to anyone. I'm wondering if the right thing to do would be to make it possible to specify which additional headers to include in notmuch-show output. This would be based on notmuch-message-headers. We already have options to include/exclude body content and html parts for much the same reasons. (Though the command-line arguments for those are incoherent at best.) It is, of course, much more work than the patch at hand. BR, Jani. > > This is required in order for the elisp variable >`notmuch-message-headers' to work. > > See this bug report: > > https://notmuchmail.org/pipermail/notmuch/2017/026069.html > --- > notmuch-show.c | 27 +++ > 1 file changed, 27 insertions(+) > > diff --git a/notmuch-show.c b/notmuch-show.c > index 21792a57..9652ed09 100644 > --- a/notmuch-show.c > +++ b/notmuch-show.c > @@ -205,6 +205,15 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage > *message, > char *recipients_string; > const char *reply_to_string; > void *local = talloc_new (sp); > +GMimeHeaderList *header_list; > +GMimeHeader *header; > +int header_count; > +/* Headers requiring some special treatment in formatting */ > +const char* special_headers[] = {"Subject", "From", "To", "Cc", "Bcc", > + "Reply-To", "In-reply-to", "References", > + "Date"}; > +const int special_header_count = sizeof(special_headers) / sizeof(char > *); > +bool special; > > sp->begin_map (sp); > > @@ -255,6 +264,24 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage > *message, > sp->string (sp, g_mime_message_get_date_string (sp, message)); > } > > +header_list = g_mime_object_get_header_list (GMIME_OBJECT(message)); > +header_count = g_mime_header_list_get_count(header_list); > + > +for (int i = 0; i < header_count; i++) { > +header = g_mime_header_list_get_header_at(header_list, i); > + special = false; > + for (int k = 0; k < special_header_count; k++) > + if (!STRNCMP_LITERAL(g_mime_header_get_name(header), > + special_headers[k])) { > + special = true; > + break; > + } > + if (special) > + continue; > + sp->map_key (sp, g_mime_header_get_name(header)); > + sp->string (sp, g_mime_header_get_value(header)); > +} > + > sp->end (sp); > talloc_free (local); > } > -- > 2.21.0 (Apple Git-122) > > ___ > notmuch mailing list > notmuch@notmuchmail.org > https://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Fix format_headers_sprinter to return all headers
Modify format_headers_sprinter so that it returns all headers in the sexp, instead of a fixed set of headers. This is required in order for the elisp variable `notmuch-message-headers' to work. See this bug report: https://notmuchmail.org/pipermail/notmuch/2017/026069.html --- notmuch-show.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/notmuch-show.c b/notmuch-show.c index 21792a57..9652ed09 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -205,6 +205,15 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage *message, char *recipients_string; const char *reply_to_string; void *local = talloc_new (sp); +GMimeHeaderList *header_list; +GMimeHeader *header; +int header_count; +/* Headers requiring some special treatment in formatting */ +const char* special_headers[] = {"Subject", "From", "To", "Cc", "Bcc", +"Reply-To", "In-reply-to", "References", +"Date"}; +const int special_header_count = sizeof(special_headers) / sizeof(char *); +bool special; sp->begin_map (sp); @@ -255,6 +264,24 @@ format_headers_sprinter (sprinter_t *sp, GMimeMessage *message, sp->string (sp, g_mime_message_get_date_string (sp, message)); } +header_list = g_mime_object_get_header_list (GMIME_OBJECT(message)); +header_count = g_mime_header_list_get_count(header_list); + +for (int i = 0; i < header_count; i++) { +header = g_mime_header_list_get_header_at(header_list, i); + special = false; + for (int k = 0; k < special_header_count; k++) + if (!STRNCMP_LITERAL(g_mime_header_get_name(header), +special_headers[k])) { + special = true; + break; + } + if (special) + continue; + sp->map_key (sp, g_mime_header_get_name(header)); + sp->string (sp, g_mime_header_get_value(header)); +} + sp->end (sp); talloc_free (local); } -- 2.21.0 (Apple Git-122) ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch