Re: [PATCH v5 0/7] notmuch search --output=sender/recipients

Subject: Re: [PATCH v5 0/7] notmuch search --output=sender/recipients

Date: Fri, 31 Oct 2014 18:46:20 +0200

To: Michal Sojka, Mark Walters, notmuch@notmuchmail.org

Cc:

From: Tomi Ollila


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

Thread: