From: Mark Walters <markwalters1009@gmail.com>
To: Michal Sojka <sojkam1@fel.cvut.cz>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 07/10] cli: search: Convert --output to keyword argument
Date: Tue, 04 Nov 2014 09:08:16 +0000 [thread overview]
Message-ID: <87wq7b2v0f.fsf@qmul.ac.uk> (raw)
In-Reply-To: <87389z4a1g.fsf@qmul.ac.uk>
> Hi
>
> On Mon, 03 Nov 2014, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
>> Now, when address related outputs are in a separate command, it makes
>> no sense to combine multiple --output options in search command line.
>> Using switch statement to handle different outputs is more readable
>> than a series of if statements.
>
> I am not keen on this change: I think the user should always be able to
> force the default output by setting command line options (which should
> protect against future changes in the default). Thus I would like to
> continue to allow --output=sender --output=recipients.
>
> I do approve of making the default do something useful but whether it
> should be both or just sender (which is much faster) is unclear to me.
Sorry my stupidity: you said "search" not "address" so this is fine.
Best wishes
Mark
>
> Best wishes
>
> Mark
>
>
>> ---
>> doc/man1/notmuch-search.rst | 3 ---
>> notmuch-search.c | 25 +++++++++++++------------
>> 2 files changed, 13 insertions(+), 15 deletions(-)
>>
>> diff --git a/doc/man1/notmuch-search.rst b/doc/man1/notmuch-search.rst
>> index 65df288..0cc2911 100644
>> --- a/doc/man1/notmuch-search.rst
>> +++ b/doc/man1/notmuch-search.rst
>> @@ -78,9 +78,6 @@ Supported options for **search** include
>> by null characters (--format=text0), as a JSON array
>> (--format=json), or as an S-Expression list (--format=sexp).
>>
>> - This option can be given multiple times to combine different
>> - 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 cbd84f5..402e860 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -593,7 +593,7 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>> int opt_index, ret;
>>
>> notmuch_opt_desc_t options[] = {
>> - { NOTMUCH_OPT_KEYWORD_FLAGS, &ctx->output, "output", 'o',
>> + { NOTMUCH_OPT_KEYWORD, &ctx->output, "output", 'o',
>> (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY },
>> { "threads", OUTPUT_THREADS },
>> { "messages", OUTPUT_MESSAGES },
>> @@ -607,13 +607,11 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>> { 0, 0, 0, 0, 0 }
>> };
>>
>> + ctx->output = OUTPUT_SUMMARY;
>> opt_index = parse_arguments (argc, argv, options, 1);
>> if (opt_index < 0)
>> return EXIT_FAILURE;
>>
>> - if (! ctx->output)
>> - ctx->output = OUTPUT_SUMMARY;
>> -
>> if (ctx->output != OUTPUT_FILES && ctx->output != OUTPUT_MESSAGES &&
>> ctx->dupe != -1) {
>> fprintf (stderr, "Error: --duplicate=N is only supported with --output=files and --output=messages.\n");
>> @@ -624,17 +622,20 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>> argc - opt_index, argv + opt_index))
>> return EXIT_FAILURE;
>>
>> - if (ctx->output == OUTPUT_SUMMARY ||
>> - ctx->output == OUTPUT_THREADS)
>> + switch (ctx->output) {
>> + case OUTPUT_SUMMARY:
>> + case OUTPUT_THREADS:
>> ret = do_search_threads (ctx);
>> - else if (ctx->output == OUTPUT_MESSAGES ||
>> - ctx->output == OUTPUT_FILES)
>> + break;
>> + case OUTPUT_MESSAGES:
>> + case OUTPUT_FILES:
>> ret = do_search_messages (ctx);
>> - else if (ctx->output == OUTPUT_TAGS)
>> + break;
>> + case OUTPUT_TAGS:
>> ret = do_search_tags (ctx);
>> - else {
>> - fprintf (stderr, "Error: the combination of outputs is not supported.\n");
>> - ret = 1;
>> + break;
>> + default:
>> + INTERNAL_ERROR ("Unexpected output");
>> }
>>
>> _notmuch_search_cleanup (ctx);
>> --
>> 2.1.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
next prev parent reply other threads:[~2014-11-04 9:08 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-03 23:50 [PATCH v2 00/10] "notmuch address" command Michal Sojka
2014-11-03 23:50 ` [PATCH v2 01/10] cli: search: Rename options to context Michal Sojka
2014-11-04 6:24 ` David Bremner
2014-11-03 23:50 ` [PATCH v2 02/10] cli: search: Move more variables into search_context_t Michal Sojka
2014-11-03 23:50 ` [PATCH v2 03/10] cli: search: Convert ctx. to ctx-> Michal Sojka
2014-11-04 6:29 ` David Bremner
2014-11-03 23:50 ` [PATCH v2 04/10] cli: search: Split notmuch_search_command to smaller functions Michal Sojka
2014-11-03 23:50 ` [PATCH v2 05/10] cli: add support for hierarchical command line option arrays Michal Sojka
2014-11-04 6:36 ` David Bremner
2014-11-04 6:38 ` David Bremner
2014-11-03 23:50 ` [PATCH v2 06/10] cli: Introduce "notmuch address" command Michal Sojka
2014-11-04 6:52 ` David Bremner
2014-11-04 9:40 ` Tomi Ollila
2014-11-04 21:59 ` Michal Sojka
2014-11-04 22:12 ` David Bremner
2014-11-04 9:04 ` Mark Walters
2014-11-04 22:15 ` Michal Sojka
2014-11-05 11:22 ` Mark Walters
2014-11-05 12:23 ` Michal Sojka
2014-11-05 12:48 ` Mark Walters
2014-11-03 23:50 ` [PATCH v2 07/10] cli: search: Convert --output to keyword argument Michal Sojka
2014-11-04 8:58 ` Mark Walters
2014-11-04 9:08 ` Mark Walters [this message]
2014-11-04 11:26 ` Michal Sojka
2014-11-03 23:50 ` [PATCH v2 08/10] cli: address: Do not output duplicate addresses Michal Sojka
2014-11-04 7:05 ` David Bremner
2014-11-04 11:36 ` Michal Sojka
2014-11-03 23:50 ` [PATCH v2 09/10] cli: address: Add --output=count Michal Sojka
2014-11-04 9:11 ` Mark Walters
2014-11-03 23:50 ` [PATCH v2 10/10] cli: address: Add --filter-by option to configure address filtering Michal Sojka
2014-11-04 9:23 ` [PATCH v2 00/10] "notmuch address" command Mark Walters
2014-11-04 20:33 ` Tomi Ollila
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=87wq7b2v0f.fsf@qmul.ac.uk \
--to=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).