[PATCH v6 0/7] notmuch search --output=sender/recipients
Mark Walters
markwalters1009 at gmail.com
Fri Oct 31 17:22:07 PDT 2014
On Fri, 31 Oct 2014, Michal Sojka <sojkam1 at fel.cvut.cz> wrote:
> Hi all,
>
> this is v6 of the search --output=address series. It obsoletes v5
> (id:1414713573-21461-1-git-send-email-sojkam1 at fel.cvut.cz).
I have tested patches 1-5 and they LGTM. +1 from me.
Best wishes
Mark
> Changes from v5 (full diff below):
>
> - Added quoting of name parts if that is necessary (pointed out by
> Mark Walters). Structured formats contain both full address
> (possibly with quoted name) and unquoted individual fields.
> - Fixed bug in --output=count --filter-by=*fold (reported by Jesse
> Rosenthal). New test was added for this case. Fixing the bug also
> resulted in simpler code :)
> - Added missing unreferencing of InternetAddressList.
>
> Changes from v4:
>
> - patch changed to commit in commit messages
> - opt->format changed to format
> - Added comments to process_* functions
> - duplicite changed to duplicate
> - check_duplicate changed to is_duplicate
> - Deduplication was split into two commits: basic deduplication
> without a command line option and configurable deduplication with
> --fiter-by.
>
> Changes from v3:
>
> - `o' renamed to `opt'.
> - Conversion of --output from keyword to keyword-flags is now a
> separate patch.
> - Structured output formats print name and address separately.
> - Added test for --format=json.
> - Changed --filter-by default to nameaddr. In v2, the default was
> addrfold, in v3 the default was no filtering at all. I believe that
> Mark's suggestion to make nameaddr the default is good trade off.
> - Added new --output=count
> - Minor style fixes
> - Few typos fixed
> - There is no way to output unfiltered (duplicite) addresses.
> Hopefully, the introduction of --output=count is sufficient
> replacement for this "feature".
>
> Cheers,
> -Michal
>
>
> Jani Nikula (1):
> cli: Add support for parsing keyword-flag arguments
>
> Michal Sojka (6):
> cli: search: Refactor passing of command line options
> cli: search: Convert --output to keyword-flag argument
> cli: search: Add --output={sender,recipients}
> cli: search: Do not output duplicate addresses
> cli: search: Add --output=count
> cli: search: Add --filter-by option to configure address filtering
>
> command-line-arguments.c | 6 +-
> command-line-arguments.h | 1 +
> completion/notmuch-completion.bash | 8 +-
> completion/notmuch-completion.zsh | 4 +-
> doc/man1/notmuch-search.rst | 66 ++++++-
> notmuch-search.c | 390 +++++++++++++++++++++++++++++--------
> test/T090-search-output.sh | 87 +++++++++
> test/T095-search-filter-by.sh | 73 +++++++
> test/T410-argument-parsing.sh | 3 +-
> test/arg-test.c | 9 +
> 10 files changed, 565 insertions(+), 82 deletions(-)
> create mode 100755 test/T095-search-filter-by.sh
>
> --
> 2.1.1
>
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 8bc80d3..a350f06 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -246,33 +246,35 @@ is_duplicate (const search_options_t *opt, GHashTable *addrs, const char *name,
> {
> notmuch_bool_t duplicate;
> char *key;
> + gchar *addrfold = NULL;
> mailbox_t *mailbox;
>
> if (opt->filter_by == FILTER_BY_ADDRFOLD ||
> - opt->filter_by == FILTER_BY_NAMEADDRFOLD) {
> - gchar *folded = g_utf8_casefold (addr, -1);
> - addr = talloc_strdup (opt->format, folded);
> - g_free (folded);
> - }
> + opt->filter_by == FILTER_BY_NAMEADDRFOLD)
> + addrfold = g_utf8_casefold (addr, -1);
> +
> switch (opt->filter_by) {
> case FILTER_BY_NAMEADDR:
> - case FILTER_BY_NAMEADDRFOLD:
> key = talloc_asprintf (opt->format, "%s <%s>", name, addr);
> break;
> + case FILTER_BY_NAMEADDRFOLD:
> + key = talloc_asprintf (opt->format, "%s <%s>", name, addrfold);
> + break;
> case FILTER_BY_NAME:
> key = talloc_strdup (opt->format, name); /* !name results in !key */
> break;
> case FILTER_BY_ADDR:
> - case FILTER_BY_ADDRFOLD:
> key = talloc_strdup (opt->format, addr);
> break;
> + case FILTER_BY_ADDRFOLD:
> + key = talloc_strdup (opt->format, addrfold);
> + break;
> default:
> INTERNAL_ERROR("invalid --filter-by flags");
> }
>
> - if (opt->filter_by == FILTER_BY_ADDRFOLD ||
> - opt->filter_by == FILTER_BY_NAMEADDRFOLD)
> - talloc_free ((char*)addr);
> + if (addrfold)
> + g_free (addrfold);
>
> if (! key)
> return FALSE;
> @@ -300,33 +302,28 @@ print_mailbox (const search_options_t *opt, const mailbox_t *mailbox)
> const char *addr = mailbox->addr;
> int count = mailbox->count;
> sprinter_t *format = opt->format;
> + InternetAddress *ia = internet_address_mailbox_new (name, addr);
> + char *name_addr;
>
> - if (format->is_text_printer) {
> - char *mailbox_str;
> + /* name_addr has the name part quoted if necessary. Compare
> + * 'John Doe <john at doe.com>' vs. '"Doe, John" <john at doe.com>' */
> + name_addr = internet_address_to_string (ia, FALSE);
>
> - if (name && *name)
> - mailbox_str = talloc_asprintf (format, "%s <%s>", name, addr);
> - else
> - mailbox_str = talloc_strdup (format, addr);
> -
> - if (! mailbox_str) {
> - fprintf (stderr, "Error: out of memory\n");
> - return;
> - }
> + if (format->is_text_printer) {
> if (count > 0) {
> format->integer (format, count);
> format->string (format, "\t");
> }
> - format->string (format, mailbox_str);
> + format->string (format, name_addr);
> format->separator (format);
> -
> - talloc_free (mailbox_str);
> } else {
> format->begin_map (format);
> format->map_key (format, "name");
> format->string (format, name);
> format->map_key (format, "address");
> format->string (format, addr);
> + format->map_key (format, "name-addr");
> + format->string (format, name_addr);
> if (count > 0) {
> format->map_key (format, "count");
> format->integer (format, count);
> @@ -334,6 +331,9 @@ print_mailbox (const search_options_t *opt, const mailbox_t *mailbox)
> format->end (format);
> format->separator (format);
> }
> +
> + g_object_unref (ia);
> + g_free (name_addr);
> }
>
> /* Print or prepare for printing addresses from InternetAddressList. */
> @@ -389,6 +389,8 @@ process_address_header (const search_options_t *opt, GHashTable *addrs, const ch
> return;
>
> process_address_list (opt, addrs, list);
> +
> + g_object_unref (list);
> }
>
> static void
> diff --git a/test/T090-search-output.sh b/test/T090-search-output.sh
> index 5a9bbc9..82380ac 100755
> --- a/test/T090-search-output.sh
> +++ b/test/T090-search-output.sh
> @@ -413,73 +413,23 @@ test_expect_equal_file OUTPUT EXPECTED
> test_begin_subtest "--output=sender --format=json"
> notmuch search --output=sender --format=json '*' >OUTPUT
> cat <<EOF >EXPECTED
> -[{"name": "François Boulogne", "address": "boulogne.f at gmail.com"},
> -{"name": "Olivier Berger", "address": "olivier.berger at it-sudparis.eu"},
> -{"name": "Chris Wilson", "address": "chris at chris-wilson.co.uk"},
> -{"name": "Carl Worth", "address": "cworth at cworth.org"},
> -{"name": "Alexander Botero-Lowry", "address": "alex.boterolowry at gmail.com"},
> -{"name": "Keith Packard", "address": "keithp at keithp.com"},
> -{"name": "Jjgod Jiang", "address": "gzjjgod at gmail.com"},
> -{"name": "Rolland Santimano", "address": "rollandsantimano at yahoo.com"},
> -{"name": "Jan Janak", "address": "jan at ryngle.com"},
> -{"name": "Stewart Smith", "address": "stewart at flamingspork.com"},
> -{"name": "Lars Kellogg-Stedman", "address": "lars at seas.harvard.edu"},
> -{"name": "Alex Botero-Lowry", "address": "alex.boterolowry at gmail.com"},
> -{"name": "Ingmar Vanhassel", "address": "ingmar at exherbo.org"},
> -{"name": "Aron Griffis", "address": "agriffis at n01se.net"},
> -{"name": "Adrian Perez de Castro", "address": "aperez at igalia.com"},
> -{"name": "Israel Herraiz", "address": "isra at herraiz.org"},
> -{"name": "Mikhail Gusarov", "address": "dottedmag at dottedmag.net"}]
> -EOF
> -test_expect_equal_file OUTPUT EXPECTED
> -
> -test_begin_subtest "--output=sender --output=count"
> -notmuch search --output=sender --output=count '*' | sort -n >OUTPUT
> -cat <<EOF >EXPECTED
> -1 Adrian Perez de Castro <aperez at igalia.com>
> -1 Aron Griffis <agriffis at n01se.net>
> -1 Chris Wilson <chris at chris-wilson.co.uk>
> -1 François Boulogne <boulogne.f at gmail.com>
> -1 Ingmar Vanhassel <ingmar at exherbo.org>
> -1 Israel Herraiz <isra at herraiz.org>
> -1 Olivier Berger <olivier.berger at it-sudparis.eu>
> -1 Rolland Santimano <rollandsantimano at yahoo.com>
> -2 Alex Botero-Lowry <alex.boterolowry at gmail.com>
> -2 Jjgod Jiang <gzjjgod at gmail.com>
> -3 Stewart Smith <stewart at flamingspork.com>
> -4 Alexander Botero-Lowry <alex.boterolowry at gmail.com>
> -4 Jan Janak <jan at ryngle.com>
> -5 Lars Kellogg-Stedman <lars at seas.harvard.edu>
> -5 Mikhail Gusarov <dottedmag at dottedmag.net>
> -7 Keith Packard <keithp at keithp.com>
> -12 Carl Worth <cworth at cworth.org>
> -EOF
> -test_expect_equal_file OUTPUT EXPECTED
> -
> -test_begin_subtest "--output=sender --output=count --format=json"
> -# Since the iteration order of GHashTable is not specified, we
> -# preprocess and sort the results to keep the order stable here.
> -notmuch search --output=sender --output=count --format=json '*' | \
> - sed -e 's/^\[//' -e 's/]$//' -e 's/,$//' | \
> - sort --field-separator=":" --key=4n --key=2 >OUTPUT
> -cat <<EOF >EXPECTED
> -{"name": "Adrian Perez de Castro", "address": "aperez at igalia.com", "count": 1}
> -{"name": "Aron Griffis", "address": "agriffis at n01se.net", "count": 1}
> -{"name": "Chris Wilson", "address": "chris at chris-wilson.co.uk", "count": 1}
> -{"name": "François Boulogne", "address": "boulogne.f at gmail.com", "count": 1}
> -{"name": "Ingmar Vanhassel", "address": "ingmar at exherbo.org", "count": 1}
> -{"name": "Israel Herraiz", "address": "isra at herraiz.org", "count": 1}
> -{"name": "Olivier Berger", "address": "olivier.berger at it-sudparis.eu", "count": 1}
> -{"name": "Rolland Santimano", "address": "rollandsantimano at yahoo.com", "count": 1}
> -{"name": "Alex Botero-Lowry", "address": "alex.boterolowry at gmail.com", "count": 2}
> -{"name": "Jjgod Jiang", "address": "gzjjgod at gmail.com", "count": 2}
> -{"name": "Stewart Smith", "address": "stewart at flamingspork.com", "count": 3}
> -{"name": "Alexander Botero-Lowry", "address": "alex.boterolowry at gmail.com", "count": 4}
> -{"name": "Jan Janak", "address": "jan at ryngle.com", "count": 4}
> -{"name": "Lars Kellogg-Stedman", "address": "lars at seas.harvard.edu", "count": 5}
> -{"name": "Mikhail Gusarov", "address": "dottedmag at dottedmag.net", "count": 5}
> -{"name": "Keith Packard", "address": "keithp at keithp.com", "count": 7}
> -{"name": "Carl Worth", "address": "cworth at cworth.org", "count": 12}
> +[{"name": "François Boulogne", "address": "boulogne.f at gmail.com", "name-addr": "François Boulogne <boulogne.f at gmail.com>"},
> +{"name": "Olivier Berger", "address": "olivier.berger at it-sudparis.eu", "name-addr": "Olivier Berger <olivier.berger at it-sudparis.eu>"},
> +{"name": "Chris Wilson", "address": "chris at chris-wilson.co.uk", "name-addr": "Chris Wilson <chris at chris-wilson.co.uk>"},
> +{"name": "Carl Worth", "address": "cworth at cworth.org", "name-addr": "Carl Worth <cworth at cworth.org>"},
> +{"name": "Alexander Botero-Lowry", "address": "alex.boterolowry at gmail.com", "name-addr": "Alexander Botero-Lowry <alex.boterolowry at gmail.com>"},
> +{"name": "Keith Packard", "address": "keithp at keithp.com", "name-addr": "Keith Packard <keithp at keithp.com>"},
> +{"name": "Jjgod Jiang", "address": "gzjjgod at gmail.com", "name-addr": "Jjgod Jiang <gzjjgod at gmail.com>"},
> +{"name": "Rolland Santimano", "address": "rollandsantimano at yahoo.com", "name-addr": "Rolland Santimano <rollandsantimano at yahoo.com>"},
> +{"name": "Jan Janak", "address": "jan at ryngle.com", "name-addr": "Jan Janak <jan at ryngle.com>"},
> +{"name": "Stewart Smith", "address": "stewart at flamingspork.com", "name-addr": "Stewart Smith <stewart at flamingspork.com>"},
> +{"name": "Lars Kellogg-Stedman", "address": "lars at seas.harvard.edu", "name-addr": "Lars Kellogg-Stedman <lars at seas.harvard.edu>"},
> +{"name": "Alex Botero-Lowry", "address": "alex.boterolowry at gmail.com", "name-addr": "Alex Botero-Lowry <alex.boterolowry at gmail.com>"},
> +{"name": "Ingmar Vanhassel", "address": "ingmar at exherbo.org", "name-addr": "Ingmar Vanhassel <ingmar at exherbo.org>"},
> +{"name": "Aron Griffis", "address": "agriffis at n01se.net", "name-addr": "Aron Griffis <agriffis at n01se.net>"},
> +{"name": "Adrian Perez de Castro", "address": "aperez at igalia.com", "name-addr": "Adrian Perez de Castro <aperez at igalia.com>"},
> +{"name": "Israel Herraiz", "address": "isra at herraiz.org", "name-addr": "Israel Herraiz <isra at herraiz.org>"},
> +{"name": "Mikhail Gusarov", "address": "dottedmag at dottedmag.net", "name-addr": "Mikhail Gusarov <dottedmag at dottedmag.net>"}]
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> @@ -487,7 +437,7 @@ test_begin_subtest "--output=recipients"
> notmuch search --output=recipients '*' >OUTPUT
> cat <<EOF >EXPECTED
> Allan McRae <allan at archlinux.org>
> -Discussion about the Arch User Repository (AUR) <aur-general at archlinux.org>
> +"Discussion about the Arch User Repository (AUR)" <aur-general at archlinux.org>
> olivier.berger at it-sudparis.eu
> notmuch at notmuchmail.org
> notmuch <notmuch at notmuchmail.org>
> @@ -501,7 +451,7 @@ notmuch search --output=sender --output=recipients '*' >OUTPUT
> cat <<EOF >EXPECTED
> François Boulogne <boulogne.f at gmail.com>
> Allan McRae <allan at archlinux.org>
> -Discussion about the Arch User Repository (AUR) <aur-general at archlinux.org>
> +"Discussion about the Arch User Repository (AUR)" <aur-general at archlinux.org>
> Olivier Berger <olivier.berger at it-sudparis.eu>
> olivier.berger at it-sudparis.eu
> Chris Wilson <chris at chris-wilson.co.uk>
> diff --git a/test/T095-search-filter-by.sh b/test/T095-search-filter-by.sh
> index 97d9a9b..15c9f77 100755
> --- a/test/T095-search-filter-by.sh
> +++ b/test/T095-search-filter-by.sh
> @@ -2,17 +2,17 @@
> test_description='duplicite address filtering in "notmuch search --output=recipients"'
> . ./test-lib.sh
>
> -add_message '[to]="Real Name <foo at example.com>, Real Name <bar at example.com>"'
> -add_message '[to]="Nickname <foo at example.com>"' '[cc]="Real Name <Bar at Example.COM>"'
> -add_message '[to]="Nickname <foo at example.com>"' '[bcc]="Real Name <Bar at Example.COM>"'
> +add_message '[to]="John Doe <foo at example.com>, John Doe <bar at example.com>"'
> +add_message '[to]="\"Doe, John\" <foo at example.com>"' '[cc]="John Doe <Bar at Example.COM>"'
> +add_message '[to]="\"Doe, John\" <foo at example.com>"' '[bcc]="John Doe <Bar at Example.COM>"'
>
> test_begin_subtest "--output=recipients"
> notmuch search --output=recipients "*" >OUTPUT
> cat <<EOF >EXPECTED
> -Real Name <foo at example.com>
> -Real Name <bar at example.com>
> -Nickname <foo at example.com>
> -Real Name <Bar at Example.COM>
> +John Doe <foo at example.com>
> +John Doe <bar at example.com>
> +"Doe, John" <foo at example.com>
> +John Doe <Bar at Example.COM>
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> @@ -20,44 +20,53 @@ test_begin_subtest "--output=recipients --filter-by=nameaddr"
> notmuch search --output=recipients --filter-by=nameaddr "*" >OUTPUT
> # The same as above
> cat <<EOF >EXPECTED
> -Real Name <foo at example.com>
> -Real Name <bar at example.com>
> -Nickname <foo at example.com>
> -Real Name <Bar at Example.COM>
> +John Doe <foo at example.com>
> +John Doe <bar at example.com>
> +"Doe, John" <foo at example.com>
> +John Doe <Bar at Example.COM>
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> test_begin_subtest "--output=recipients --filter-by=name"
> notmuch search --output=recipients --filter-by=name "*" >OUTPUT
> cat <<EOF >EXPECTED
> -Real Name <foo at example.com>
> -Nickname <foo at example.com>
> +John Doe <foo at example.com>
> +"Doe, John" <foo at example.com>
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> test_begin_subtest "--output=recipients --filter-by=addr"
> notmuch search --output=recipients --filter-by=addr "*" >OUTPUT
> cat <<EOF >EXPECTED
> -Real Name <foo at example.com>
> -Real Name <bar at example.com>
> -Real Name <Bar at Example.COM>
> +John Doe <foo at example.com>
> +John Doe <bar at example.com>
> +John Doe <Bar at Example.COM>
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> test_begin_subtest "--output=recipients --filter-by=addrfold"
> notmuch search --output=recipients --filter-by=addrfold "*" >OUTPUT
> cat <<EOF >EXPECTED
> -Real Name <foo at example.com>
> -Real Name <bar at example.com>
> +John Doe <foo at example.com>
> +John Doe <bar at example.com>
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> test_begin_subtest "--output=recipients --filter-by=nameaddrfold"
> notmuch search --output=recipients --filter-by=nameaddrfold "*" >OUTPUT
> cat <<EOF >EXPECTED
> -Real Name <foo at example.com>
> -Real Name <bar at example.com>
> -Nickname <foo at example.com>
> +John Doe <foo at example.com>
> +John Doe <bar at example.com>
> +"Doe, John" <foo at example.com>
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "--output=recipients --filter-by=nameaddrfold --output=count"
> +notmuch search --output=recipients --filter-by=nameaddrfold --output=count "*" | sort -n >OUTPUT
> +cat <<EOF >EXPECTED
> +1 John Doe <foo at example.com>
> +2 "Doe, John" <foo at example.com>
> +3 John Doe <bar at example.com>
> EOF
> test_expect_equal_file OUTPUT EXPECTED
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list