Re: [PATCH] Fix format_headers_sprinter to return all headers (v2)

2019-11-10 Thread Tomi Ollila
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)

2019-11-10 Thread Johan Parin
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)

2019-11-10 Thread David Edmondson
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

2019-11-10 Thread Jani Nikula
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

2019-11-10 Thread Jani Nikula
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)

2019-11-09 Thread Tomi Ollila


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)

2019-11-09 Thread Johan Parin


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)

2019-11-09 Thread Johan Parin
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

2019-11-09 Thread Johan Parin


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

2019-11-09 Thread David Bremner
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

2019-11-09 Thread Jani Nikula
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

2019-11-09 Thread Johan Parin
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