unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: Tomi Ollila <tomi.ollila@iki.fi>
To: Michal Sojka <sojkam1@fel.cvut.cz>,
	Mark Walters <markwalters1009@gmail.com>,
	notmuch@notmuchmail.org
Subject: Re: [PATCH v5 0/7] notmuch search --output=sender/recipients
Date: Fri, 31 Oct 2014 18:46:20 +0200	[thread overview]
Message-ID: <m24mukyyoz.fsf@guru.guru-group.fi> (raw)
In-Reply-To: <87vbn06yry.fsf@steelpick.2x.cz>

On Fri, Oct 31 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:

> On Fri, Oct 31 2014, Mark Walters wrote:
>> My only query is in the text output: should the name part be printed as
>> a quoted string. For example currently I get a line of the form
>>
>> Bloggs, Fred <fred@example.com>
>
> Good point.

There has been some discussion on this issue on IRC channel, and the
opinion of most (seen so far) is that output the parts without quoting
(i.e. just like done in this patch series)...

Taken the other example from Mark's earlyer email:

Bloggs <the king>, Fred <fred@example.com>

echo '^^^' | sed 's/.*</</' would leave only the address part (with <>:s) (*)

echo '^^^' | sed 's/ <[^<]*$//' would leave only the name part

and regexp '\(.*\) <\(.*\)>' or pcre-compatible /(.*)\s<(.*)>/
would capture name & address parts...

(all of the above untested, though;)

In case instead of 'name <addr>' there is only 'addr', then above
sed lines return the same (full) string and the regexps just don't
match.

There were some suggestions how the text output could be changed on IRC;
if anyone wishes to bring those forward, please do so :D

So, IMO, this issue is not showstopper in this series (if anything is);
patches 1-6 looks good to me on paper, but I have not tested those yet.

Tomi

(*) echo '^^^' | sed 's/.*<//; s/>.*//' would drop <>'s from first example


>
> On Fri, Oct 31 2014, Mark Walters wrote:
>> Hi
>>
>> I attach a patch which does the quoting for real names but I don't know
>> if we want it: it changes (example taken from the test suite)
>>
>> François Boulogne to
>>
>> =?iso-8859-1?q?Fran=E7ois?= Boulogne
>>
>> (which is what was in the original email)
>>
>> Plausibly the best thing is just to leave the series as is, so the
>> text output is readable and parseable in the common cases.
>>
>> Anyway the patch is attached if anyone wants to experiment.
>>
>> Best wishes
>>
>> Mark
>>
>> From 53b1ced2d6a9fbbba93448325f795e6b99faa240 Mon Sep 17 00:00:00 2001
>> From: Mark Walters <markwalters1009@gmail.com>
>> Date: Fri, 31 Oct 2014 10:11:40 +0000
>> Subject: [PATCH] search: quote real names for output=sender/recipient in text
>>  format
>>
>> This quotes the real name (when gmime thinks appropriate) for the text
>> output. For the other outputs the real name is separate from the
>> address so the consumer can do any quoting necessary.
>> ---
>>  notmuch-search.c           |    8 ++++----
>>  test/T090-search-output.sh |    8 ++++----
>>  2 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/notmuch-search.c b/notmuch-search.c
>> index eae749a..8eac161 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -47,6 +47,7 @@ typedef struct {
>>  typedef struct {
>>      const char *name;
>>      const char *addr;
>> +    const char *string;
>>  } mailbox_t;
>>  
>>  /* Return two stable query strings that identify exactly the matched
>> @@ -255,15 +256,13 @@ print_mailbox (const search_options_t *opt, const mailbox_t *mailbox)
>>  {
>>      const char *name = mailbox->name;
>>      const char *addr = mailbox->addr;
>> +    const char *string = mailbox->string;
>>      sprinter_t *format = opt->format;
>>  
>>      if (format->is_text_printer) {
>>  	char *mailbox_str;
>>  
>> -	if (name && *name)
>> -	    mailbox_str = talloc_asprintf (format, "%s <%s>", name, addr);
>> -	else
>> -	    mailbox_str = talloc_strdup (format, addr);
>> +	mailbox_str = talloc_strdup (format, string);
>>  
>>  	if (! mailbox_str) {
>>  	    fprintf (stderr, "Error: out of memory\n");
>> @@ -309,6 +308,7 @@ process_address_list (const search_options_t *opt, GHashTable *addrs,
>>  	    mailbox_t mbx = {
>>  		.name = internet_address_get_name (address),
>>  		.addr = internet_address_mailbox_get_addr (mailbox),
>> +		.string = internet_address_to_string (address, TRUE),
>
> I'd prefer having the second parameter (encode) FALSE. This will still
> add quotes when necessary, but does not encode non-ascii characters so
> the result would be human readable.
>
> Another question is whether to add .string to mailbox_t. In this patch
> it doesn't matter, but if --output=count patch will be merged, this
> would mean that memory consumption doubles, because with --output=count
> the addresses are kept in memory and printed only after the search is
> completed. It would be therefore better to construct a new
> InternetAddressMailbox from name and addr in print_mailbox() and perform
> the conversion to string there. Thoughts?
>
> Thanks,
> -Michal
>
>>  	    };
>>  
>>  	    if (is_duplicate (opt, addrs, mbx.name, mbx.addr))
>> diff --git a/test/T090-search-output.sh b/test/T090-search-output.sh
>> index 841a721..776e3f4 100755
>> --- a/test/T090-search-output.sh
>> +++ b/test/T090-search-output.sh
>> @@ -390,7 +390,7 @@ test_expect_equal_file OUTPUT EXPECTED
>>  test_begin_subtest "--output=sender"
>>  notmuch search --output=sender '*' >OUTPUT
>>  cat <<EOF >EXPECTED
>> -François Boulogne <boulogne.f@gmail.com>
>> +=?iso-8859-1?q?Fran=E7ois?= Boulogne <boulogne.f@gmail.com>
>>  Olivier Berger <olivier.berger@it-sudparis.eu>
>>  Chris Wilson <chris@chris-wilson.co.uk>
>>  Carl Worth <cworth@cworth.org>
>> @@ -437,7 +437,7 @@ test_begin_subtest "--output=recipients"
>>  notmuch search --output=recipients '*' >OUTPUT
>>  cat <<EOF >EXPECTED
>>  Allan McRae <allan@archlinux.org>
>> -Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
>> +"Discussion about the Arch User Repository (AUR)" <aur-general@archlinux.org>
>>  olivier.berger@it-sudparis.eu
>>  notmuch@notmuchmail.org
>>  notmuch <notmuch@notmuchmail.org>
>> @@ -449,9 +449,9 @@ test_expect_equal_file OUTPUT EXPECTED
>>  test_begin_subtest "--output=sender --output=recipients"
>>  notmuch search --output=sender --output=recipients '*' >OUTPUT
>>  cat <<EOF >EXPECTED
>> -François Boulogne <boulogne.f@gmail.com>
>> +=?iso-8859-1?q?Fran=E7ois?= Boulogne <boulogne.f@gmail.com>
>>  Allan McRae <allan@archlinux.org>
>> -Discussion about the Arch User Repository (AUR) <aur-general@archlinux.org>
>> +"Discussion about the Arch User Repository (AUR)" <aur-general@archlinux.org>
>>  Olivier Berger <olivier.berger@it-sudparis.eu>
>>  olivier.berger@it-sudparis.eu
>>  Chris Wilson <chris@chris-wilson.co.uk>
>> -- 
>> 1.7.10.4
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

  reply	other threads:[~2014-10-31 16:46 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-30 23:59 [PATCH v5 0/7] notmuch search --output=sender/recipients Michal Sojka
2014-10-30 23:59 ` [PATCH v5 1/7] cli: search: Refactor passing of command line options Michal Sojka
2014-10-30 23:59 ` [PATCH v5 2/7] cli: Add support for parsing keyword-flag arguments Michal Sojka
2014-10-30 23:59 ` [PATCH v5 3/7] cli: search: Convert --output to keyword-flag argument Michal Sojka
2014-10-30 23:59 ` [PATCH v5 4/7] cli: search: Add --output={sender,recipients} Michal Sojka
2014-10-30 23:59 ` [PATCH v5 5/7] cli: search: Do not output duplicate addresses Michal Sojka
2014-10-30 23:59 ` [PATCH v5 6/7] cli: search: Add --output=count Michal Sojka
2014-10-30 23:59 ` [PATCH v5 7/7] cli: search: Add --filter-by option to configure address filtering Michal Sojka
2014-10-31  8:54 ` [PATCH v5 0/7] notmuch search --output=sender/recipients Mark Walters
2014-10-31 10:20   ` Mark Walters
2014-10-31 15:32     ` Michal Sojka
2014-10-31 16:46       ` Tomi Ollila [this message]
2014-10-31 16:54         ` Mark Walters
2014-10-31 16:47       ` Mark Walters
2014-10-31 15:03 ` Jesse Rosenthal
2014-10-31 15:43   ` Michal Sojka

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=m24mukyyoz.fsf@guru.guru-group.fi \
    --to=tomi.ollila@iki.fi \
    --cc=markwalters1009@gmail.com \
    --cc=notmuch@notmuchmail.org \
    --cc=sojkam1@fel.cvut.cz \
    /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).