On Tue, Nov 04 2014, Mark Walters wrote: > Hi > > On Mon, 03 Nov 2014, Michal Sojka <sojkam1 at 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).
You can do this even with this patch. > Thus I would like to continue to allow --output=sender > --output=recipients. This is allowed in notmuch address. This patch modifies only notmuch search. Currently it makes no sense to run --output=files --output=tags or other output combinations. > 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. Does this comment refer to 7/10 or to 6/10? I'd say it refers to the following line from 6/10. search_context.output = OUTPUT_SENDER | OUTPUT_RECIPIENTS; Even there you can override the default by command line option. This get executed only if no --output appears on the command line. Thanks -Michal > > 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 at notmuchmail.org >> http://notmuchmail.org/mailman/listinfo/notmuch