Hi Mark, I mostly agree with your points mentioned in this and other your emails. I'll prepare v4 based on that.
On Wed, Oct 22 2014, Mark Walters wrote: > On Sun, 12 Oct 2014, Michal Sojka <sojkam1 at fel.cvut.cz> wrote: >> The new outputs allow printing senders, recipients or both of matching >> messages. The --output option is converted from "keyword" argument to >> "flags" argument, which means that the user can use --output=sender and >> --output=recipients simultaneously, to print both. Other combinations >> produce an error. >> >> This code based on a patch from Jani Nikula. >> --- >> completion/notmuch-completion.bash | 2 +- >> completion/notmuch-completion.zsh | 3 +- >> doc/man1/notmuch-search.rst | 22 +++++++- >> notmuch-search.c | 110 >> ++++++++++++++++++++++++++++++++++--- >> test/T090-search-output.sh | 64 +++++++++++++++++++++ >> 5 files changed, 189 insertions(+), 12 deletions(-) >> >> diff --git a/completion/notmuch-completion.bash >> b/completion/notmuch-completion.bash >> index 0571dc9..cfbd389 100644 >> --- a/completion/notmuch-completion.bash >> +++ b/completion/notmuch-completion.bash >> @@ -294,7 +294,7 @@ _notmuch_search() >> return >> ;; >> --output) >> - COMPREPLY=( $( compgen -W "summary threads messages files tags" -- >> "${cur}" ) ) >> + COMPREPLY=( $( compgen -W "summary threads messages files tags >> sender recipients" -- "${cur}" ) ) >> return >> ;; >> --sort) >> diff --git a/completion/notmuch-completion.zsh >> b/completion/notmuch-completion.zsh >> index 67a9aba..3e52a00 100644 >> --- a/completion/notmuch-completion.zsh >> +++ b/completion/notmuch-completion.zsh >> @@ -52,7 +52,8 @@ _notmuch_search() >> _arguments -s : \ >> '--max-threads=[display only the first x threads from the search >> results]:number of threads to show: ' \ >> '--first=[omit the first x threads from the search results]:number of >> threads to omit: ' \ >> - '--sort=[sort results]:sorting:((newest-first\:"reverse chronological >> order" oldest-first\:"chronological order"))' >> + '--sort=[sort results]:sorting:((newest-first\:"reverse chronological >> order" oldest-first\:"chronological order"))' \ >> + '--output=[select what to output]:output:((summary threads messages >> files tags sender recipients))' >> } >> >> _notmuch() >> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst >> index 90160f2..c9d38b1 100644 >> --- a/doc/man1/notmuch-search.rst >> +++ b/doc/man1/notmuch-search.rst >> @@ -35,7 +35,7 @@ Supported options for **search** include >> intended for programs that invoke **notmuch(1)** internally. If >> omitted, the latest supported version will be used. >> >> - ``--output=(summary|threads|messages|files|tags)`` >> + ``--output=(summary|threads|messages|files|tags|sender|recipients)`` >> >> **summary** >> Output a summary of each thread with any message matching >> @@ -78,6 +78,26 @@ Supported options for **search** include >> by null characters (--format=text0), as a JSON array >> (--format=json), or as an S-Expression list (--format=sexp). >> >> + **sender** >> + Output all addresses from the *From* header that appear on >> + any message matching the search terms, either one per line >> + (--format=text), separated by null characters >> + (--format=text0), as a JSON array (--format=json), or as >> + an S-Expression list (--format=sexp). >> + >> + Note: Searching for **sender** should be much faster than >> + searching for **recipients**, because sender addresses are >> + cached directly in the database whereas other addresses >> + need to be fetched from message files. >> + >> + **recipients** >> + Like **sender** but for addresses from *To*, *Cc* and >> + *Bcc* headers. >> + >> + This option can be given multiple times to combine different >> + outputs. Curently, this is only supported for **sender** and >> + **recipients** outputs. >> + >> ``--sort=``\ (**newest-first**\ \|\ **oldest-first**) >> This option can be used to present results in either >> chronological order (**oldest-first**) or reverse chronological >> diff --git a/notmuch-search.c b/notmuch-search.c >> index 5ac2a26..74588f8 100644 >> --- a/notmuch-search.c >> +++ b/notmuch-search.c >> @@ -23,11 +23,14 @@ >> #include "string-util.h" >> >> typedef enum { >> - OUTPUT_SUMMARY, >> - OUTPUT_THREADS, >> - OUTPUT_MESSAGES, >> - OUTPUT_FILES, >> - OUTPUT_TAGS >> + OUTPUT_SUMMARY = 1 << 0, >> + OUTPUT_THREADS = 1 << 1, >> + OUTPUT_MESSAGES = 1 << 2, >> + OUTPUT_FILES = 1 << 3, >> + OUTPUT_TAGS = 1 << 4, >> + OUTPUT_SENDER = 1 << 5, >> + OUTPUT_RECIPIENTS = 1 << 6, >> + OUTPUT_ADDRESSES = OUTPUT_SENDER | OUTPUT_RECIPIENTS, > > I think I would drop the OUTPUT_ADDRESSES enum as the parser no longer > uses it (and replace the one use by OUTPUT_SENDER | OUTPUT_RECIPIENTS > below). As mentioned elsewhere, this is required to suppress the following warning. notmuch-search.c:634:5: warning: case value ?96? not in enumerated type ?output_t? [-Wswitch] > >> } output_t; >> >> typedef struct { >> @@ -220,6 +223,67 @@ do_search_threads (search_options_t *o) >> return 0; >> } >> >> +static void >> +print_address_list (const search_options_t *o, InternetAddressList *list) >> +{ >> + InternetAddress *address; >> + int i; >> + >> + for (i = 0; i < internet_address_list_length (list); i++) { >> + address = internet_address_list_get_address (list, i); >> + if (INTERNET_ADDRESS_IS_GROUP (address)) { >> + InternetAddressGroup *group; >> + InternetAddressList *group_list; >> + >> + group = INTERNET_ADDRESS_GROUP (address); >> + group_list = internet_address_group_get_members (group); >> + if (group_list == NULL) >> + continue; >> + >> + print_address_list (o, group_list); >> + } else { >> + InternetAddressMailbox *mailbox; >> + const char *name; >> + const char *addr; >> + char *full_address; >> + >> + mailbox = INTERNET_ADDRESS_MAILBOX (address); >> + >> + name = internet_address_get_name (address); >> + addr = internet_address_mailbox_get_addr (mailbox); >> + >> + if (name && *name) >> + full_address = talloc_asprintf (o->format, "%s <%s>", name, >> addr); >> + else >> + full_address = talloc_strdup (o->format, addr); >> + >> + if (!full_address) { >> + fprintf (stderr, "Error: out of memory\n"); >> + break; >> + } >> + o->format->string (o->format, full_address); >> + o->format->separator (o->format); >> + >> + talloc_free (full_address); >> + } >> + } >> +} >> + >> +static void >> +print_address_string (const search_options_t *o, const char *recipients) >> +{ >> + InternetAddressList *list; >> + >> + if (recipients == NULL) >> + return; >> + >> + list = internet_address_list_parse_string (recipients); >> + if (list == NULL) >> + return; >> + >> + print_address_list (o, list); >> +} >> + >> static int >> do_search_messages (search_options_t *o) >> { >> @@ -266,11 +330,29 @@ do_search_messages (search_options_t *o) >> >> notmuch_filenames_destroy( filenames ); >> >> - } else { /* output == OUTPUT_MESSAGES */ >> + } else if (o->output == OUTPUT_MESSAGES) { >> format->set_prefix (format, "id"); >> format->string (format, >> notmuch_message_get_message_id (message)); >> format->separator (format); >> + } else { >> + if (o->output & OUTPUT_SENDER) { >> + const char *addrs; >> + >> + addrs = notmuch_message_get_header (message, "from"); >> + print_address_string (o, addrs); >> + } >> + >> + if (o->output & OUTPUT_RECIPIENTS) { >> + const char *hdrs[] = { "to", "cc", "bcc" }; >> + const char *addrs; >> + size_t j; >> + >> + for (j = 0; j < ARRAY_SIZE (hdrs); j++) { >> + addrs = notmuch_message_get_header (message, hdrs[j]); >> + print_address_string (o, addrs); >> + } >> + } >> } >> >> notmuch_message_destroy (message); >> @@ -337,7 +419,7 @@ notmuch_search_command (notmuch_config_t *config, int >> argc, char *argv[]) >> notmuch_database_t *notmuch; >> search_options_t o = { >> .sort = NOTMUCH_SORT_NEWEST_FIRST, >> - .output = OUTPUT_SUMMARY, >> + .output = 0, >> .offset = 0, >> .limit = -1, /* unlimited */ >> .dupe = -1, > > It is slightly unfortunate that all the other defaults are defined here > but the default output is after the option parsing but I can't see a > nice way round that. I only mention it in case you (or anyone else) can > see a nice way round this. Solution would to have --output as NOTMUCH_OPT_KEYWORD rather than NOTMUCH_OPT_KEYWORD_FLAGS as it was in v2. > >> @@ -366,10 +448,12 @@ notmuch_search_command (notmuch_config_t *config, int >> argc, char *argv[]) >> { "text0", NOTMUCH_FORMAT_TEXT0 }, >> { 0, 0 } } }, >> { NOTMUCH_OPT_INT, ¬much_format_version, "format-version", 0, 0 }, >> - { NOTMUCH_OPT_KEYWORD, &o.output, "output", 'o', >> + { NOTMUCH_OPT_KEYWORD_FLAGS, &o.output, "output", 'o', >> (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY }, >> { "threads", OUTPUT_THREADS }, >> { "messages", OUTPUT_MESSAGES }, >> + { "sender", OUTPUT_SENDER }, >> + { "recipients", OUTPUT_RECIPIENTS }, >> { "files", OUTPUT_FILES }, >> { "tags", OUTPUT_TAGS }, >> { 0, 0 } } }, >> @@ -389,6 +473,9 @@ notmuch_search_command (notmuch_config_t *config, int >> argc, char *argv[]) >> if (opt_index < 0) >> return EXIT_FAILURE; >> >> + if (! o.output) >> + o.output = OUTPUT_SUMMARY; >> + >> switch (format_sel) { >> case NOTMUCH_FORMAT_TEXT: >> o.format = sprinter_text_create (config, stdout); >> @@ -455,18 +542,23 @@ notmuch_search_command (notmuch_config_t *config, int >> argc, char *argv[]) >> } >> >> switch (o.output) { >> - default: >> case OUTPUT_SUMMARY: >> case OUTPUT_THREADS: >> ret = do_search_threads (&o); >> break; >> case OUTPUT_MESSAGES: >> + case OUTPUT_SENDER: >> + case OUTPUT_RECIPIENTS: >> + case OUTPUT_ADDRESSES: > > This is the one use of OUTPUT_ADDRESSES that I would replace with > OUTPUT_SENDER | OUTPUT_RECIPIENTS See above. -Michal