From: Jani Nikula <jani@nikula.org>
To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
Subject: Re: [PATCH] Fix format_headers_sprinter to return all headers (v2)
Date: Sun, 10 Nov 2019 11:41:31 +0200 [thread overview]
Message-ID: <87h83cax10.fsf@nikula.org> (raw)
In-Reply-To: <m2imns1xue.fsf@guru.guru-group.fi>
On Sun, 10 Nov 2019, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> 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
Yes, something like this is what I had in mind.
But this needs to be carefully thought out. Obviously we need a default
set. Is it possible to remove headers from the default set? Does
--message-headers=Foo override the default set too? Should we force the
inclusion of certain "fundamental" headers? Etc. As I said, the
notmuch-show interface is a bit confusing already, better not make it
more confusing.
> 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]);
Yes, but please use ARRAY_SIZE() instead. ;)
BR,
Jani.
>
>> 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
next prev parent reply other threads:[~2019-11-10 9:41 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-09 22:13 [PATCH] Fix format_headers_sprinter to return all headers (v2) Johan Parin
2019-11-09 22:21 ` Johan Parin
2019-11-09 22:36 ` Tomi Ollila
2019-11-10 9:41 ` Jani Nikula [this message]
2019-11-10 14:03 ` David Edmondson
2019-11-10 17:12 ` Johan Parin
2019-11-10 20:10 ` Tomi Ollila
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87h83cax10.fsf@nikula.org \
--to=jani@nikula.org \
--cc=notmuch@notmuchmail.org \
--cc=tomi.ollila@iki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).