From: Michal Sojka <sojkam1@fel.cvut.cz>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 06/10] cli: Introduce "notmuch address" command
In-Reply-To: <87zjc72v79.fsf@qmul.ac.uk>
References: <1415058622-21162-1-git-send-email-sojkam1@fel.cvut.cz>
	<1415058622-21162-7-git-send-email-sojkam1@fel.cvut.cz>
	<87zjc72v79.fsf@qmul.ac.uk>
User-Agent: Notmuch/0.18.2+157~ga00d359 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-pc-linux-gnu)
Date: Tue, 04 Nov 2014 23:15:38 +0100
Message-ID: <87y4rqliid.fsf@steelpick.2x.cz>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Tue, Nov 04 2014, Mark Walters wrote:
> On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>> This moves address-related functionality from search command to the
>> new address command. The implementation shares almost all code and
>> some command line options.
>>
>> Options --offset and --limit were intentionally not included in the
>> address command, because they refer to messages numbers, which users
>> do not see in the output. This could confuse users because, for
>> example, they could see more addresses in the output that what was
>> specified with --limit. This functionality can be correctly
>> reimplemented for addresses later.
>
> I am not sure about this: we already have this anomaly for output=files
> say. Also I can imagine calling notmuch address --limit=1000 ... to get
> a bunch of recent addresses quickly and I really am wanting to look at
> 1000 messages, not collect 1000 addresses.

I think that one of the reasons for having the new "address" command is
to have cleaner user interface. And including "anomalies" doesn't sound
like a way to achieve this. I think that now you can use "date:" query
to limit the search.

I volunteer to implement "address --limit" properly after 0.19. This
should be easy.

-Michal

> Additionally, the 1000 message approach makes sense when we start
> deduping whereas 1000 authors becomes unclear.
>
>>
>> This was inspired by a patch from Jani Nikula.
>> ---
>>  completion/notmuch-completion.bash |  42 ++++++++++++++-
>>  completion/notmuch-completion.zsh  |  10 +++-
>>  doc/man1/notmuch-address.rst       |  99 ++++++++++++++++++++++++++++++++++++
>>  doc/man1/notmuch-search.rst        |  20 +-------
>>  doc/man1/notmuch.rst               |   7 +--
>>  notmuch-client.h                   |   3 ++
>>  notmuch-search.c                   | 101 +++++++++++++++++++++++++------------
>>  notmuch.c                          |   2 +
>>  8 files changed, 228 insertions(+), 56 deletions(-)
>>  create mode 100644 doc/man1/notmuch-address.rst
>>
>> diff --git a/completion/notmuch-completion.bash b/completion/notmuch-completion.bash
>> index cfbd389..94ea2d5 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 sender recipients" -- "${cur}" ) )
>> +	    COMPREPLY=( $( compgen -W "summary threads messages files tags" -- "${cur}" ) )
>>  	    return
>>  	    ;;
>>  	--sort)
>> @@ -320,6 +320,44 @@ _notmuch_search()
>>      esac
>>  }
>>  
>> +_notmuch_address()
>> +{
>> +    local cur prev words cword split
>> +    _init_completion -s || return
>> +
>> +    $split &&
>> +    case "${prev}" in
>> +	--format)
>> +	    COMPREPLY=( $( compgen -W "json sexp text text0" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--output)
>> +	    COMPREPLY=( $( compgen -W "sender recipients" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--sort)
>> +	    COMPREPLY=( $( compgen -W "newest-first oldest-first" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +	--exclude)
>> +	    COMPREPLY=( $( compgen -W "true false flag all" -- "${cur}" ) )
>> +	    return
>> +	    ;;
>> +    esac
>> +
>> +    ! $split &&
>> +    case "${cur}" in
>> +	-*)
>> +	    local options="--format= --output= --sort= --exclude="
>> +	    compopt -o nospace
>> +	    COMPREPLY=( $(compgen -W "$options" -- ${cur}) )
>> +	    ;;
>> +	*)
>> +	    _notmuch_search_terms
>> +	    ;;
>> +    esac
>> +}
>> +
>>  _notmuch_show()
>>  {
>>      local cur prev words cword split
>> @@ -393,7 +431,7 @@ _notmuch_tag()
>>  
>>  _notmuch()
>>  {
>> -    local _notmuch_commands="compact config count dump help insert new reply restore search setup show tag"
>> +    local _notmuch_commands="compact config count dump help insert new reply restore search address setup show tag"
>>      local arg cur prev words cword split
>>  
>>      # require bash-completion with _init_completion
>> diff --git a/completion/notmuch-completion.zsh b/completion/notmuch-completion.zsh
>> index 3e52a00..c606b75 100644
>> --- a/completion/notmuch-completion.zsh
>> +++ b/completion/notmuch-completion.zsh
>> @@ -10,6 +10,7 @@ _notmuch_commands()
>>      'setup:interactively set up notmuch for first use'
>>      'new:find and import any new message to the database'
>>      'search:search for messages matching the search terms, display matching threads as results'
>> +    'address:get addresses from messages matching the given search terms'
>>      'reply:constructs a reply template for a set of messages'
>>      'show:show all messages matching the search terms'
>>      'tag:add or remove tags for all messages matching the search terms'
>> @@ -53,7 +54,14 @@ _notmuch_search()
>>      '--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"))' \
>> -    '--output=[select what to output]:output:((summary threads messages files tags sender recipients))'
>> +    '--output=[select what to output]:output:((summary threads messages files tags))'
>> +}
>> +
>> +_notmuch_address()
>> +{
>> +  _arguments -s : \
>> +    '--sort=[sort results]:sorting:((newest-first\:"reverse chronological order" oldest-first\:"chronological order"))' \
>> +    '--output=[select what to output]:output:((sender recipients))'
>>  }
>>  
>>  _notmuch()
>> diff --git a/doc/man1/notmuch-address.rst b/doc/man1/notmuch-address.rst
>> new file mode 100644
>> index 0000000..8109f11
>> --- /dev/null
>> +++ b/doc/man1/notmuch-address.rst
>> @@ -0,0 +1,99 @@
>> +===============
>> +notmuch-address
>> +===============
>> +
>> +SYNOPSIS
>> +========
>> +
>> +**notmuch** **address** [*option* ...] <*search-term*> ...
>> +
>> +DESCRIPTION
>> +===========
>> +
>> +Search for messages matching the given search terms, and display the
>> +addresses from them.
>> +
>> +See **notmuch-search-terms(7)** for details of the supported syntax for
>> +<search-terms>.
>> +
>> +Supported options for **address** include
>> +
>> +    ``--format=``\ (**json**\ \|\ **sexp**\ \|\ **text**\ \|\ **text0**)
>> +        Presents the results in either JSON, S-Expressions, newline
>> +        character separated plain-text (default), or null character
>> +        separated plain-text (compatible with **xargs(1)** -0 option
>> +        where available).
>> +
>> +    ``--format-version=N``
>> +        Use the specified structured output format version. This is
>> +        intended for programs that invoke **notmuch(1)** internally. If
>> +        omitted, the latest supported version will be used.
>> +
>> +    ``--output=(sender|recipients)``
>> +
>> +        Controls which information appears in the output. This option
>> +	can be given multiple times to combine different outputs.
>> +	Omitting this option is equivalent to
>> +	--output=sender --output=recipients.
>> +
>> +	**sender**
>> +            Output all addresses from the *From* header.
>> +
>> +	    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**
>> +            Output all addresses from the *To*, *Cc* and *Bcc*
>> +            headers.
>> +
>> +    ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>> +        This option can be used to present results in either
>> +        chronological order (**oldest-first**) or reverse chronological
>> +        order (**newest-first**).
>> +
>> +        By default, results will be displayed in reverse chronological
>> +        order, (that is, the newest results will be displayed first).
>> +
>> +    ``--exclude=(true|false|all|flag)``
>> +        A message is called "excluded" if it matches at least one tag in
>> +        search.tag\_exclude that does not appear explicitly in the
>> +        search terms. This option specifies whether to omit excluded
>> +        messages in the search process.
>> +
>> +        The default value, **true**, prevents excluded messages from
>> +        matching the search terms.
>> +
>> +        **all** additionally prevents excluded messages from appearing
>> +        in displayed results, in effect behaving as though the excluded
>> +        messages do not exist.
>> +
>> +        **false** allows excluded messages to match search terms and
>> +        appear in displayed results. Excluded messages are still marked
>> +        in the relevant outputs.
>> +
>> +        **flag** only has an effect when ``--output=summary``. The
>> +        output is almost identical to **false**, but the "match count"
>> +        is the number of matching non-excluded messages in the thread,
>> +        rather than the number of matching messages.
>
> The exclude text needs updating as flag makes no sense for the address
> command. 
>
> Best wishes 
>
> Mark
>
>> +EXIT STATUS
>> +===========
>> +
>> +This command supports the following special exit status codes
>> +
>> +``20``
>> +    The requested format version is too old.
>> +
>> +``21``
>> +    The requested format version is too new.
>> +
>> +SEE ALSO
>> +========
>> +
>> +**notmuch(1)**, **notmuch-config(1)**, **notmuch-count(1)**,
>> +**notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
>> +**notmuch-new(1)**, **notmuch-reply(1)**, **notmuch-restore(1)**,
>> +**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**,
>> +***notmuch-search(1)**
>> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
>> index 8110086..65df288 100644
>> --- a/doc/man1/notmuch-search.rst
>> +++ b/doc/man1/notmuch-search.rst
>> @@ -78,25 +78,8 @@ 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. Currently, this is only supported for **sender** and
>> -	**recipients** outputs.
>> +	outputs.
>>  
>>      ``--sort=``\ (**newest-first**\ \|\ **oldest-first**)
>>          This option can be used to present results in either
>> @@ -173,3 +156,4 @@ SEE ALSO
>>  **notmuch-dump(1)**, **notmuch-hooks(5)**, **notmuch-insert(1)**,
>>  **notmuch-new(1)**, **notmuch-reply(1)**, **notmuch-restore(1)**,
>>  **notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**
>> +***notmuch-address(1)**
>> diff --git a/doc/man1/notmuch.rst b/doc/man1/notmuch.rst
>> index 9710294..98590a4 100644
>> --- a/doc/man1/notmuch.rst
>> +++ b/doc/man1/notmuch.rst
>> @@ -88,8 +88,8 @@ Several of the notmuch commands accept search terms with a common
>>  syntax. See **notmuch-search-terms**\ (7) for more details on the
>>  supported syntax.
>>  
>> -The **search**, **show** and **count** commands are used to query the
>> -email database.
>> +The **search**, **show**, **address** and **count** commands are used
>> +to query the email database.
>>  
>>  The **reply** command is useful for preparing a template for an email
>>  reply.
>> @@ -128,7 +128,8 @@ SEE ALSO
>>  **notmuch-config(1)**, **notmuch-count(1)**, **notmuch-dump(1)**,
>>  **notmuch-hooks(5)**, **notmuch-insert(1)**, **notmuch-new(1)**,
>>  **notmuch-reply(1)**, **notmuch-restore(1)**, **notmuch-search(1)**,
>> -**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**
>> +**notmuch-search-terms(7)**, **notmuch-show(1)**, **notmuch-tag(1)**,
>> +***notmuch-address(1)**
>>  
>>  The notmuch website: **http://notmuchmail.org**
>>  
>> diff --git a/notmuch-client.h b/notmuch-client.h
>> index e1efbe0..5e0d475 100644
>> --- a/notmuch-client.h
>> +++ b/notmuch-client.h
>> @@ -199,6 +199,9 @@ int
>>  notmuch_search_command (notmuch_config_t *config, int argc, char *argv[]);
>>  
>>  int
>> +notmuch_address_command (notmuch_config_t *config, int argc, char *argv[]);
>> +
>> +int
>>  notmuch_setup_command (notmuch_config_t *config, int argc, char *argv[]);
>>  
>>  int
>> diff --git a/notmuch-search.c b/notmuch-search.c
>> index f115359..cbd84f5 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -23,17 +23,18 @@
>>  #include "string-util.h"
>>  
>>  typedef enum {
>> +    /* Search command */
>>      OUTPUT_SUMMARY	= 1 << 0,
>>      OUTPUT_THREADS	= 1 << 1,
>>      OUTPUT_MESSAGES	= 1 << 2,
>>      OUTPUT_FILES	= 1 << 3,
>>      OUTPUT_TAGS		= 1 << 4,
>> +
>> +    /* Address command */
>>      OUTPUT_SENDER	= 1 << 5,
>>      OUTPUT_RECIPIENTS	= 1 << 6,
>>  } output_t;
>>  
>> -#define OUTPUT_ADDRESS_FLAGS (OUTPUT_SENDER | OUTPUT_RECIPIENTS)
>> -
>>  typedef enum {
>>      NOTMUCH_FORMAT_JSON,
>>      NOTMUCH_FORMAT_TEXT,
>> @@ -554,51 +555,55 @@ _notmuch_search_cleanup (search_context_t *ctx)
>>      talloc_free (ctx->format);
>>  }
>>  
>> +static search_context_t search_context = {
>> +    .format_sel = NOTMUCH_FORMAT_TEXT,
>> +    .exclude = NOTMUCH_EXCLUDE_TRUE,
>> +    .sort = NOTMUCH_SORT_NEWEST_FIRST,
>> +    .output = 0,
>> +    .offset = 0,
>> +    .limit = -1, /* unlimited */
>> +    .dupe = -1,
>> +};
>> +
>> +static const notmuch_opt_desc_t common_options[] = {
>> +    { NOTMUCH_OPT_KEYWORD, &search_context.sort, "sort", 's',
>> +      (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
>> +			      { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
>> +			      { 0, 0 } } },
>> +    { NOTMUCH_OPT_KEYWORD, &search_context.format_sel, "format", 'f',
>> +      (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
>> +			      { "sexp", NOTMUCH_FORMAT_SEXP },
>> +			      { "text", NOTMUCH_FORMAT_TEXT },
>> +			      { "text0", NOTMUCH_FORMAT_TEXT0 },
>> +			      { 0, 0 } } },
>> +    { NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
>> +    { NOTMUCH_OPT_KEYWORD, &search_context.exclude, "exclude", 'x',
>> +      (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
>> +			      { "false", NOTMUCH_EXCLUDE_FALSE },
>> +			      { "flag", NOTMUCH_EXCLUDE_FLAG },
>> +			      { "all", NOTMUCH_EXCLUDE_ALL },
>> +			      { 0, 0 } } },
>> +    { 0, 0, 0, 0, 0 }
>> +};
>> +
>>  int
>>  notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  {
>> -    search_context_t search_context = {
>> -	.format_sel = NOTMUCH_FORMAT_TEXT,
>> -	.exclude = NOTMUCH_EXCLUDE_TRUE,
>> -	.sort = NOTMUCH_SORT_NEWEST_FIRST,
>> -	.output = 0,
>> -	.offset = 0,
>> -	.limit = -1, /* unlimited */
>> -	.dupe = -1,
>> -    };
>>      search_context_t *ctx = &search_context;
>>      int opt_index, ret;
>>  
>>      notmuch_opt_desc_t options[] = {
>> -	{ NOTMUCH_OPT_KEYWORD, &ctx->sort, "sort", 's',
>> -	  (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST },
>> -				  { "newest-first", NOTMUCH_SORT_NEWEST_FIRST },
>> -				  { 0, 0 } } },
>> -	{ NOTMUCH_OPT_KEYWORD, &ctx->format_sel, "format", 'f',
>> -	  (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
>> -				  { "sexp", NOTMUCH_FORMAT_SEXP },
>> -				  { "text", NOTMUCH_FORMAT_TEXT },
>> -				  { "text0", NOTMUCH_FORMAT_TEXT0 },
>> -				  { 0, 0 } } },
>> -	{ NOTMUCH_OPT_INT, &notmuch_format_version, "format-version", 0, 0 },
>>  	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->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 } } },
>> -        { NOTMUCH_OPT_KEYWORD, &ctx->exclude, "exclude", 'x',
>> -          (notmuch_keyword_t []){ { "true", NOTMUCH_EXCLUDE_TRUE },
>> -                                  { "false", NOTMUCH_EXCLUDE_FALSE },
>> -                                  { "flag", NOTMUCH_EXCLUDE_FLAG },
>> -                                  { "all", NOTMUCH_EXCLUDE_ALL },
>> -                                  { 0, 0 } } },
>>  	{ NOTMUCH_OPT_INT, &ctx->offset, "offset", 'O', 0 },
>>  	{ NOTMUCH_OPT_INT, &ctx->limit, "limit", 'L', 0  },
>>  	{ NOTMUCH_OPT_INT, &ctx->dupe, "duplicate", 'D', 0  },
>> +	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
>>  	{ 0, 0, 0, 0, 0 }
>>      };
>>  
>> @@ -623,8 +628,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  	ctx->output == OUTPUT_THREADS)
>>  	ret = do_search_threads (ctx);
>>      else if (ctx->output == OUTPUT_MESSAGES ||
>> -	     ctx->output == OUTPUT_FILES ||
>> -	     (ctx->output & OUTPUT_ADDRESS_FLAGS && !(ctx->output & ~OUTPUT_ADDRESS_FLAGS)))
>> +	     ctx->output == OUTPUT_FILES)
>>  	ret = do_search_messages (ctx);
>>      else if (ctx->output == OUTPUT_TAGS)
>>  	ret = do_search_tags (ctx);
>> @@ -637,3 +641,36 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  
>>      return ret ? EXIT_FAILURE : EXIT_SUCCESS;
>>  }
>> +
>> +int
>> +notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
>> +{
>> +    search_context_t *ctx = &search_context;
>> +    int opt_index, ret;
>> +
>> +    notmuch_opt_desc_t options[] = {
>> +	{ NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
>> +	  (notmuch_keyword_t []){ { "sender", OUTPUT_SENDER },
>> +				  { "recipients", OUTPUT_RECIPIENTS },
>> +				  { 0, 0 } } },
>> +	{ NOTMUCH_OPT_INHERIT, &common_options, NULL, 0, 0 },
>> +	{ 0, 0, 0, 0, 0 }
>> +    };
>> +
>> +    opt_index = parse_arguments (argc, argv, options, 1);
>> +    if (opt_index < 0)
>> +	return EXIT_FAILURE;
>> +
>> +    if (! ctx->output)
>> +	search_context.output = OUTPUT_SENDER | OUTPUT_RECIPIENTS;
>> +
>> +    if (_notmuch_search_prepare (ctx, config,
>> +				 argc - opt_index, argv + opt_index))
>> +	return EXIT_FAILURE;
>> +
>> +    ret = do_search_messages (ctx);
>> +
>> +    _notmuch_search_cleanup (ctx);
>> +
>> +    return ret ? EXIT_FAILURE : EXIT_SUCCESS;
>> +}
>> diff --git a/notmuch.c b/notmuch.c
>> index dcda039..0fac099 100644
>> --- a/notmuch.c
>> +++ b/notmuch.c
>> @@ -54,6 +54,8 @@ static command_t commands[] = {
>>        "Add a new message into the maildir and notmuch database." },
>>      { "search", notmuch_search_command, FALSE,
>>        "Search for messages matching the given search terms." },
>> +    { "address", notmuch_address_command, FALSE,
>> +      "Get addresses from messages matching the given search terms." },
>>      { "show", notmuch_show_command, FALSE,
>>        "Show all messages matching the search terms." },
>>      { "count", notmuch_count_command, FALSE,
>> -- 
>> 2.1.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch