On Sat, 26 May 2012, Peter Wang <novalazy@gmail.com> wrote: > On Tue, 24 Apr 2012 10:11:13 +0100, Mark Walters <markwalters1009@gmail.com> wrote: >> The --entire-thread option in notmuch-show.c defaults to true when >> format=json. Previously there was no way to turn this off. This patch >> makes it respect --entire-thread=false. >> >> The one subtlety is that we initialise a notmuch_bool_t to -1 to >> indicate that the option parsing has not set it. This allows the code >> to distinguish between the option being omitted from the command line, >> and the option being set to false on the command line. >> --- >> notmuch-show.c | 16 ++++++++++++++-- >> 1 files changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/notmuch-show.c b/notmuch-show.c >> index 0d21f1a..48551bb 100644 >> --- a/notmuch-show.c >> +++ b/notmuch-show.c >> @@ -996,7 +996,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) >> char *query_string; >> int opt_index, ret; >> const notmuch_show_format_t *format = &format_text; >> - notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE }; >> + >> + /* We abuse the notmuch_bool_t variable params.entire-thread by >> + * setting it to -1 to denote that the command line parsing has >> + * not set it. We ensure it is set to TRUE or FALSE before passing >> + * it to any other function.*/ >> + notmuch_show_params_t params = { .part = -1, .entire_thread = -1 }; >> + >> int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; >> notmuch_bool_t verify = FALSE; >> int exclude = EXCLUDE_TRUE; > > Hi Mark, > > As an alternative to the abuse, could you just treat it as with exclude, > using an enum with three values (TRUE|FALSE|DEFAULT)? > Then set params.entire_thread afterwards. The reason I haven't done this is that the current command line parser does not allow keyword options to take default values: in other words --entire-thread without an "=<something>" would not be allowed. It is easy to change the keyword parsing code to allow this: I include a first draft of such a patch below. This would allow the solution you suggest and thus avoid the hack/abuse. What do people think? Best wishes Mark --- command-line-arguments.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/command-line-arguments.c b/command-line-arguments.c index 76b185f..d40c7e6 100644 --- a/command-line-arguments.c +++ b/command-line-arguments.c @@ -11,10 +11,16 @@ */ static notmuch_bool_t -_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, const char *arg_str) { +_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) { const notmuch_keyword_t *keywords = arg_desc->keywords; + if (next == 0) { +/* No keyword given so return first option as default */ + *((int *)arg_desc->output_var) = keywords->value; + return TRUE; + } + while (keywords->name) { if (strcmp (arg_str, keywords->name) == 0) { if (arg_desc->output_var) { @@ -99,7 +105,8 @@ parse_option (const char *arg, */ if (next != '=' && next != ':' && next != 0) return FALSE; if (next == 0) { - if (try->opt_type != NOTMUCH_OPT_BOOLEAN) + if (try->opt_type != NOTMUCH_OPT_BOOLEAN && + try->opt_type != NOTMUCH_OPT_KEYWORD) return FALSE; } else { if (value[0] == 0) return FALSE; @@ -110,7 +117,7 @@ parse_option (const char *arg, switch (try->opt_type) { case NOTMUCH_OPT_KEYWORD: - return _process_keyword_arg (try, value); + return _process_keyword_arg (try, next, value); break; case NOTMUCH_OPT_BOOLEAN: return _process_boolean_arg (try, next, value); -- 1.7.9.1