* [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN
@ 2012-03-08 22:15 Mark Walters
2012-03-08 22:15 ` [PATCH 1/2] " Mark Walters
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Mark Walters @ 2012-03-08 22:15 UTC (permalink / raw)
To: notmuch
The first patch adds a new command line parsing option
NOTMUCH_OPT_INT_OR_BOOLEAN for command line parsing which accepts
--verbose=3 and --verbose with the latter setting verbose to 1. It
also allows --verbose=0 so (with a little caller support) the user can
turn off boolean options.
It also means that extra options can be added to the command line
programs in a backwards compatible manner (e.g. if --verbose already
exists we could add --verbose=2).
The second patch uses this to make the --entire-thread option to
notmuch-show a NOTMUCH_OPT_INT_OR_BOOLEAN. In particular this allows
the caller to disable --entire-thread (with --entire-thread=0) when
format=json.
Best wishes
Mark
Mark Walters (2):
cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN
cli: make --entire-thread option notmuch-show.c INT_OR_BOOLEAN
command-line-arguments.c | 29 +++++++++++++++++++++++++++--
command-line-arguments.h | 3 +++
notmuch-show.c | 10 ++++++++--
3 files changed, 38 insertions(+), 4 deletions(-)
--
1.7.9.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN 2012-03-08 22:15 [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN Mark Walters @ 2012-03-08 22:15 ` Mark Walters 2012-03-08 22:15 ` [PATCH 2/2] cli: make --entire-thread option notmuch-show.c INT_OR_BOOLEAN Mark Walters 2012-03-09 0:48 ` [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN Jani Nikula 2 siblings, 0 replies; 11+ messages in thread From: Mark Walters @ 2012-03-08 22:15 UTC (permalink / raw) To: notmuch Allow the option NOTMUCH_OPT_INT_OR_BOOLEAN for command line parsing which accepts --verbose=3 and --verbose with the latter setting verbose to 1. It also allows --verbose=0 so (with a little caller support) user can turn off boolean options. This means that extra options can be added to the command line programs in a backwards compatible manner. --- command-line-arguments.c | 29 +++++++++++++++++++++++++++-- command-line-arguments.h | 3 +++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/command-line-arguments.c b/command-line-arguments.c index e711414..99e13c6 100644 --- a/command-line-arguments.c +++ b/command-line-arguments.c @@ -4,6 +4,22 @@ #include "error_util.h" #include "command-line-arguments.h" +/* A helper for parsing an int to a boolean */ +notmuch_bool_t +notmuch_int_to_boolean (int i) +{ + switch (i) { + case 1: + return TRUE; + case 0: + return FALSE; + default: + INTERNAL_ERROR ("Non-boolean value %d", i); + /* UNREACHED */ + return FALSE; + } +} + /* Search the array of keywords for a given argument, assigning the output variable to the corresponding value. Return FALSE if nothing @@ -72,6 +88,7 @@ parse_option (const char *arg, if (try->name && strncmp (arg, try->name, strlen (try->name)) == 0) { char next = arg[strlen (try->name)]; const char *value= arg+strlen(try->name)+1; + enum notmuch_opt_type opt_type = try->opt_type; char *endptr; @@ -79,7 +96,14 @@ parse_option (const char *arg, * delimiter, and a non-zero length value */ - if (try->opt_type != NOTMUCH_OPT_BOOLEAN) { + if (opt_type == NOTMUCH_OPT_INT_OR_BOOLEAN) { + if (next != 0) + opt_type = NOTMUCH_OPT_INT; + else + opt_type = NOTMUCH_OPT_BOOLEAN; + } + + if (opt_type != NOTMUCH_OPT_BOOLEAN) { if (next != '=' && next != ':') return FALSE; if (value[0] == 0) return FALSE; } else { @@ -89,7 +113,7 @@ parse_option (const char *arg, if (try->output_var == NULL) INTERNAL_ERROR ("output pointer NULL for option %s", try->name); - switch (try->opt_type) { + switch (opt_type) { case NOTMUCH_OPT_KEYWORD: return _process_keyword_arg (try, value); break; @@ -107,6 +131,7 @@ parse_option (const char *arg, break; case NOTMUCH_OPT_POSITION: case NOTMUCH_OPT_END: + case NOTMUCH_OPT_INT_OR_BOOLEAN: /* should be dealt with above */ default: INTERNAL_ERROR ("unknown or unhandled option type %d", try->opt_type); /*UNREACHED*/ diff --git a/command-line-arguments.h b/command-line-arguments.h index de1734a..a2fc545 100644 --- a/command-line-arguments.h +++ b/command-line-arguments.h @@ -6,6 +6,7 @@ enum notmuch_opt_type { NOTMUCH_OPT_END = 0, NOTMUCH_OPT_BOOLEAN, /* --verbose */ + NOTMUCH_OPT_INT_OR_BOOLEAN, /* --verbose or --verbose=1 */ NOTMUCH_OPT_INT, /* --frob=8 */ NOTMUCH_OPT_KEYWORD, /* --format=raw|json|text */ NOTMUCH_OPT_STRING, /* --file=/tmp/gnarf.txt */ @@ -76,5 +77,7 @@ parse_position_arg (const char *arg, int position_arg_index, const notmuch_opt_desc_t* options); +notmuch_bool_t +notmuch_int_to_boolean (int i); #endif -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] cli: make --entire-thread option notmuch-show.c INT_OR_BOOLEAN 2012-03-08 22:15 [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN Mark Walters 2012-03-08 22:15 ` [PATCH 1/2] " Mark Walters @ 2012-03-08 22:15 ` Mark Walters 2012-03-09 0:48 ` [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN Jani Nikula 2 siblings, 0 replies; 11+ messages in thread From: Mark Walters @ 2012-03-08 22:15 UTC (permalink / raw) To: notmuch Make the --entire-thread option notmuch-show.c NOTMUCH_OPT_INT_OR_BOOLEAN. In particular this means a caller can turn off entire-thread (by specifying --entire-thread=0) when format=json. (This was not previously possible.) --- notmuch-show.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index 05d51b2..f0c640f 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -985,6 +985,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) const notmuch_show_format_t *format = &format_text; notmuch_show_params_t params = { .part = -1 }; int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; + int entire_thread = -1; notmuch_bool_t verify = FALSE; notmuch_bool_t no_exclude = FALSE; @@ -996,7 +997,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) { "raw", NOTMUCH_FORMAT_RAW }, { 0, 0 } } }, { NOTMUCH_OPT_INT, ¶ms.part, "part", 'p', 0 }, - { NOTMUCH_OPT_BOOLEAN, ¶ms.entire_thread, "entire-thread", 't', 0 }, + { NOTMUCH_OPT_INT_OR_BOOLEAN, &entire_thread, "entire-thread", 't', 0 }, { NOTMUCH_OPT_BOOLEAN, ¶ms.decrypt, "decrypt", 'd', 0 }, { NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 }, { NOTMUCH_OPT_BOOLEAN, &no_exclude, "no-exclude", 'n', 0 }, @@ -1020,7 +1021,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) switch (format_sel) { case NOTMUCH_FORMAT_JSON: format = &format_json; - params.entire_thread = TRUE; + if (entire_thread == -1) + entire_thread = 1; break; case NOTMUCH_FORMAT_TEXT: format = &format_text; @@ -1042,6 +1044,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) params.raw = TRUE; break; } + /* Set default to not entire_thread; JSON case dealt with above */ + if (entire_thread == -1) + entire_thread = 0; + params.entire_thread = notmuch_int_to_boolean (entire_thread); if (params.decrypt || verify) { #ifdef GMIME_ATLEAST_26 -- 1.7.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN 2012-03-08 22:15 [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN Mark Walters 2012-03-08 22:15 ` [PATCH 1/2] " Mark Walters 2012-03-08 22:15 ` [PATCH 2/2] cli: make --entire-thread option notmuch-show.c INT_OR_BOOLEAN Mark Walters @ 2012-03-09 0:48 ` Jani Nikula 2012-03-09 5:11 ` Mark Walters 2 siblings, 1 reply; 11+ messages in thread From: Jani Nikula @ 2012-03-09 0:48 UTC (permalink / raw) To: Mark Walters, notmuch On Thu, 8 Mar 2012 22:15:42 +0000, Mark Walters <markwalters1009@gmail.com> wrote: > The first patch adds a new command line parsing option > NOTMUCH_OPT_INT_OR_BOOLEAN for command line parsing which accepts > --verbose=3 and --verbose with the latter setting verbose to 1. It > also allows --verbose=0 so (with a little caller support) the user can > turn off boolean options. > > It also means that extra options can be added to the command line > programs in a backwards compatible manner (e.g. if --verbose already > exists we could add --verbose=2). > > The second patch uses this to make the --entire-thread option to > notmuch-show a NOTMUCH_OPT_INT_OR_BOOLEAN. In particular this allows > the caller to disable --entire-thread (with --entire-thread=0) when > format=json. I'm afraid I find both of the patches a bit hacky. The first because it's more about optional arguments to options than int-or-bool. The second because it's more about detecting whether the user has provided an option than int-or-bool. For booleans, I think the notmuch style would be to allow --foo=true and --foo=false in addition to just --foo (which implies true) so you could also disable booleans. I think this would be fairly simple to implement, and would require no changes to option parser users. With --entire-thread the bigger problem is detecting whether the option was specified by the user or not. It would be great if the option parser could provide this information, but it might take a while to get there... In the mean time, I think I would rather see a well commented local hack here initializing the notmuch_bool_t params.entire_thread to -1, and changing it to TRUE or FALSE if not already done so by parse_arguments(). BR, Jani. > > Best wishes > > Mark > > Mark Walters (2): > cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN > cli: make --entire-thread option notmuch-show.c INT_OR_BOOLEAN > > command-line-arguments.c | 29 +++++++++++++++++++++++++++-- > command-line-arguments.h | 3 +++ > notmuch-show.c | 10 ++++++++-- > 3 files changed, 38 insertions(+), 4 deletions(-) > > -- > 1.7.9.1 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN 2012-03-09 0:48 ` [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN Jani Nikula @ 2012-03-09 5:11 ` Mark Walters 2012-03-09 22:33 ` [PATCH 0/3] argument parsing additions Jani Nikula 0 siblings, 1 reply; 11+ messages in thread From: Mark Walters @ 2012-03-09 5:11 UTC (permalink / raw) To: Jani Nikula, notmuch Hi On Fri, 09 Mar 2012 02:48:35 +0200, Jani Nikula <jani@nikula.org> wrote: > On Thu, 8 Mar 2012 22:15:42 +0000, Mark Walters <markwalters1009@gmail.com> wrote: > > The first patch adds a new command line parsing option > > NOTMUCH_OPT_INT_OR_BOOLEAN for command line parsing which accepts > > --verbose=3 and --verbose with the latter setting verbose to 1. It > > also allows --verbose=0 so (with a little caller support) the user can > > turn off boolean options. > > > > It also means that extra options can be added to the command line > > programs in a backwards compatible manner (e.g. if --verbose already > > exists we could add --verbose=2). > > > > The second patch uses this to make the --entire-thread option to > > notmuch-show a NOTMUCH_OPT_INT_OR_BOOLEAN. In particular this allows > > the caller to disable --entire-thread (with --entire-thread=0) when > > format=json. > > I'm afraid I find both of the patches a bit hacky. The first because > it's more about optional arguments to options than int-or-bool. The > second because it's more about detecting whether the user has provided > an option than int-or-bool. Yes to both of these (although I don't think of it has hacky). The second of these is the important consideration. > For booleans, I think the notmuch style would be to allow --foo=true and > --foo=false in addition to just --foo (which implies true) so you could > also disable booleans. I think this would be fairly simple to implement, > and would require no changes to option parser users. > > With --entire-thread the bigger problem is detecting whether the option > was specified by the user or not. It would be great if the option parser > could provide this information, but it might take a while to get > there... In the mean time, I think I would rather see a well commented > local hack here initializing the notmuch_bool_t params.entire_thread to > -1, and changing it to TRUE or FALSE if not already done so by > parse_arguments(). I think these need to be considered together. There are three important possibilities for a boolean option foo: 1) foo not mentioned 2) --foo=false and 3) --foo=true (--foo as an alias) but the parser only has a boolean to store this in. We could overload the boolean as you suggest (it is really an int so should be safe) but that does seem hacky. That was why I decided to make the parser set an int rather than a boolean. Since there are not very many OPT_BOOLEANs currently in the code I think my decision to allow general ints to be returned is a needless extension, but I do think we wish to allow the 3 states of UNSET, FALSE and TRUE. Otherwise we limit any future boolean options to always have the same default regardless of other settings. If people are happy with setting a notmuch_bool_t to -1 (for unset) then that is definitely the simplest option. Or if people think that default boolean options should not depend on other options then we can just add this hack for --entire-thread. Best wishes Mark ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 0/3] argument parsing additions 2012-03-09 5:11 ` Mark Walters @ 2012-03-09 22:33 ` Jani Nikula 2012-03-09 22:33 ` [PATCH 1/3] command-line-arguments: allow true and false keywords for booleans Jani Nikula ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: Jani Nikula @ 2012-03-09 22:33 UTC (permalink / raw) To: notmuch, markwalters1009 Hi Mark - I'm not sure which is worse, criticizing or rewriting other people's patches. I already did the former, and now I'm doing the latter. Apologies for both. I didn't really mean to write these patches, but it turned out to be more fun writing a proper reply in C than in English. Patch 1 adds --arg=true and --arg=false support for booleans. It's not strictly required for the --entire-thread support in patch 3, which uses the extension of keyword arguments from patch 2, but it's for consistency across boolean arguments. Please let me know what you think. BR, Jani. Jani Nikula (3): command-line-arguments: allow true and false keywords for booleans command-line-arguments: support keyword arguments with default value cli: allow switching off entire thread mode in notmuch show json format command-line-arguments.c | 45 +++++++++++++++++++++++++++++++++++++++------ command-line-arguments.h | 1 + notmuch-show.c | 12 ++++++++++-- 3 files changed, 50 insertions(+), 8 deletions(-) -- 1.7.5.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] command-line-arguments: allow true and false keywords for booleans 2012-03-09 22:33 ` [PATCH 0/3] argument parsing additions Jani Nikula @ 2012-03-09 22:33 ` Jani Nikula 2012-03-09 22:33 ` [PATCH 2/3] command-line-arguments: support keyword arguments with default value Jani Nikula ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Jani Nikula @ 2012-03-09 22:33 UTC (permalink / raw) To: notmuch, markwalters1009 Add support for --arg=true and --arg=false for NOTMUCH_OPT_BOOLEAN arguments to be able to disable a boolean argument. Plain --arg remains unchanged, meaning true. Signed-off-by: Jani Nikula <jani@nikula.org> --- command-line-arguments.c | 36 ++++++++++++++++++++++++++++++------ 1 files changed, 30 insertions(+), 6 deletions(-) diff --git a/command-line-arguments.c b/command-line-arguments.c index e711414..1bdb881 100644 --- a/command-line-arguments.c +++ b/command-line-arguments.c @@ -28,6 +28,27 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, const char *arg_str) { return FALSE; } +static notmuch_bool_t +_process_boolean_arg (const notmuch_opt_desc_t *arg_desc, const char *arg_str) +{ + notmuch_bool_t value = TRUE; + + if (arg_str) { + if (strcmp (arg_str, "true") == 0) { + value = TRUE; + } else if (strcmp (arg_str, "false") == 0) { + value = FALSE; + } else { + fprintf (stderr, "unknown boolean: %s\n", arg_str); + return FALSE; + } + } + + *((notmuch_bool_t *)arg_desc->output_var) = value; + + return TRUE; +} + /* Search for the {pos_arg_index}th position argument, return FALSE if that does not exist. @@ -79,11 +100,15 @@ parse_option (const char *arg, * delimiter, and a non-zero length value */ - if (try->opt_type != NOTMUCH_OPT_BOOLEAN) { - if (next != '=' && next != ':') return FALSE; - if (value[0] == 0) return FALSE; + if (next == '=' || next == ':') { + if (value[0] == '\0') + return FALSE; + } else if (next == '\0') { + value = NULL; + if (try->opt_type != NOTMUCH_OPT_BOOLEAN) + return FALSE; } else { - if (next != 0) return FALSE; + return FALSE; } if (try->output_var == NULL) @@ -94,8 +119,7 @@ parse_option (const char *arg, return _process_keyword_arg (try, value); break; case NOTMUCH_OPT_BOOLEAN: - *((notmuch_bool_t *)try->output_var) = TRUE; - return TRUE; + return _process_boolean_arg (try, value); break; case NOTMUCH_OPT_INT: *((int *)try->output_var) = strtol (value, &endptr, 10); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] command-line-arguments: support keyword arguments with default value 2012-03-09 22:33 ` [PATCH 0/3] argument parsing additions Jani Nikula 2012-03-09 22:33 ` [PATCH 1/3] command-line-arguments: allow true and false keywords for booleans Jani Nikula @ 2012-03-09 22:33 ` Jani Nikula 2012-03-09 22:33 ` [PATCH 3/3] cli: allow switching off entire thread mode in notmuch show json format Jani Nikula 2012-03-10 11:23 ` [PATCH 0/3] argument parsing additions Mark Walters 3 siblings, 0 replies; 11+ messages in thread From: Jani Nikula @ 2012-03-09 22:33 UTC (permalink / raw) To: notmuch, markwalters1009 Add NOTMUCH_OPT_KEYWORD_DEFAULT to support plain --arg in addition to --arg=value. The value to use is the first in the list of keywords. Signed-off-by: Jani Nikula <jani@nikula.org> --- command-line-arguments.c | 11 ++++++++++- command-line-arguments.h | 1 + 2 files changed, 11 insertions(+), 1 deletions(-) diff --git a/command-line-arguments.c b/command-line-arguments.c index 1bdb881..2a3646f 100644 --- a/command-line-arguments.c +++ b/command-line-arguments.c @@ -15,6 +15,13 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, const char *arg_str) { const notmuch_keyword_t *keywords = arg_desc->keywords; + if (arg_str == NULL) { + /* no keyword specified, use the first keyword as default */ + if (arg_desc->output_var) + *((int *)arg_desc->output_var) = keywords->value; + return TRUE; + } + while (keywords->name) { if (strcmp (arg_str, keywords->name) == 0) { if (arg_desc->output_var) { @@ -105,7 +112,8 @@ parse_option (const char *arg, return FALSE; } else if (next == '\0') { value = NULL; - if (try->opt_type != NOTMUCH_OPT_BOOLEAN) + if (try->opt_type != NOTMUCH_OPT_BOOLEAN && + try->opt_type != NOTMUCH_OPT_KEYWORD_DEFAULT) return FALSE; } else { return FALSE; @@ -116,6 +124,7 @@ parse_option (const char *arg, switch (try->opt_type) { case NOTMUCH_OPT_KEYWORD: + case NOTMUCH_OPT_KEYWORD_DEFAULT: return _process_keyword_arg (try, value); break; case NOTMUCH_OPT_BOOLEAN: diff --git a/command-line-arguments.h b/command-line-arguments.h index de1734a..d70c84c 100644 --- a/command-line-arguments.h +++ b/command-line-arguments.h @@ -8,6 +8,7 @@ enum notmuch_opt_type { NOTMUCH_OPT_BOOLEAN, /* --verbose */ NOTMUCH_OPT_INT, /* --frob=8 */ NOTMUCH_OPT_KEYWORD, /* --format=raw|json|text */ + NOTMUCH_OPT_KEYWORD_DEFAULT,/* --format[=raw|json] */ NOTMUCH_OPT_STRING, /* --file=/tmp/gnarf.txt */ NOTMUCH_OPT_POSITION /* notmuch dump pos_arg */ }; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] cli: allow switching off entire thread mode in notmuch show json format 2012-03-09 22:33 ` [PATCH 0/3] argument parsing additions Jani Nikula 2012-03-09 22:33 ` [PATCH 1/3] command-line-arguments: allow true and false keywords for booleans Jani Nikula 2012-03-09 22:33 ` [PATCH 2/3] command-line-arguments: support keyword arguments with default value Jani Nikula @ 2012-03-09 22:33 ` Jani Nikula 2012-03-10 11:23 ` [PATCH 0/3] argument parsing additions Mark Walters 3 siblings, 0 replies; 11+ messages in thread From: Jani Nikula @ 2012-03-09 22:33 UTC (permalink / raw) To: notmuch, markwalters1009 Previously --format=json implied --entire-thread with no way to switch it off. Add support for --entire-thread=false usable with json format. Signed-off-by: Jani Nikula <jani@nikula.org> --- notmuch-show.c | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index 05d51b2..19e0119 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -985,6 +985,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) const notmuch_show_format_t *format = &format_text; notmuch_show_params_t params = { .part = -1 }; int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; + int entire_thread = -1; notmuch_bool_t verify = FALSE; notmuch_bool_t no_exclude = FALSE; @@ -996,7 +997,10 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) { "raw", NOTMUCH_FORMAT_RAW }, { 0, 0 } } }, { NOTMUCH_OPT_INT, ¶ms.part, "part", 'p', 0 }, - { NOTMUCH_OPT_BOOLEAN, ¶ms.entire_thread, "entire-thread", 't', 0 }, + { NOTMUCH_OPT_KEYWORD_DEFAULT, &entire_thread, "entire-thread", 't', + (notmuch_keyword_t []){ { "true", TRUE }, + { "false", FALSE }, + { 0, 0 } } }, { NOTMUCH_OPT_BOOLEAN, ¶ms.decrypt, "decrypt", 'd', 0 }, { NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 }, { NOTMUCH_OPT_BOOLEAN, &no_exclude, "no-exclude", 'n', 0 }, @@ -1020,7 +1024,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) switch (format_sel) { case NOTMUCH_FORMAT_JSON: format = &format_json; - params.entire_thread = TRUE; + if (entire_thread == -1) + params.entire_thread = TRUE; break; case NOTMUCH_FORMAT_TEXT: format = &format_text; @@ -1043,6 +1048,9 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) break; } + if (entire_thread != -1) + params.entire_thread = entire_thread; + if (params.decrypt || verify) { #ifdef GMIME_ATLEAST_26 /* TODO: GMimePasswordRequestFunc */ -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] argument parsing additions 2012-03-09 22:33 ` [PATCH 0/3] argument parsing additions Jani Nikula ` (2 preceding siblings ...) 2012-03-09 22:33 ` [PATCH 3/3] cli: allow switching off entire thread mode in notmuch show json format Jani Nikula @ 2012-03-10 11:23 ` Mark Walters 2012-03-11 19:26 ` Jani Nikula 3 siblings, 1 reply; 11+ messages in thread From: Mark Walters @ 2012-03-10 11:23 UTC (permalink / raw) To: Jani Nikula, notmuch On Sat, 10 Mar 2012 00:33:27 +0200, Jani Nikula <jani@nikula.org> wrote: > Hi Mark - > > I'm not sure which is worse, criticizing or rewriting other people's > patches. I already did the former, and now I'm doing the > latter. Apologies for both. I didn't really mean to write these patches, > but it turned out to be more fun writing a proper reply in C than in > English. > > Patch 1 adds --arg=true and --arg=false support for booleans. It's not > strictly required for the --entire-thread support in patch 3, which uses > the extension of keyword arguments from patch 2, but it's for > consistency across boolean arguments. Hi I like patch 1: I have an almost identical to my version (in the series I just sent to the list id:"1331377533-30262-1-git-send-email-markwalters1009@gmail.com> X-Mailer: git-send-email 1.7.9.1"). I am not sure about patch 2 and patch 3. Do you have a use case for --option except when option is a boolean? Otherwise I think I prefer either my approach (abusing a notmuch_bool_t) or just adding an option NOTMUCH_OPT_BOOLEAN_AS_INT which does boolean parsing but returns an int. I guess I am saying that I think allowing boolean options which can sometimes default to true and sometimes to false is more useful than allowing --option for arbitrary keywords (*). What do you think? Best wishes Mark (*) Indeed, I was thinking of the former as a possibility for the exclude code, but I am erring towards just using keywords so I can allow more options as you suggested. > > Please let me know what you think. > > BR, > Jani. > > > Jani Nikula (3): > command-line-arguments: allow true and false keywords for booleans > command-line-arguments: support keyword arguments with default value > cli: allow switching off entire thread mode in notmuch show json > format > > command-line-arguments.c | 45 +++++++++++++++++++++++++++++++++++++++------ > command-line-arguments.h | 1 + > notmuch-show.c | 12 ++++++++++-- > 3 files changed, 50 insertions(+), 8 deletions(-) > > -- > 1.7.5.4 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] argument parsing additions 2012-03-10 11:23 ` [PATCH 0/3] argument parsing additions Mark Walters @ 2012-03-11 19:26 ` Jani Nikula 0 siblings, 0 replies; 11+ messages in thread From: Jani Nikula @ 2012-03-11 19:26 UTC (permalink / raw) To: Mark Walters, notmuch On Sat, 10 Mar 2012 11:23:15 +0000, Mark Walters <markwalters1009@gmail.com> wrote: > On Sat, 10 Mar 2012 00:33:27 +0200, Jani Nikula <jani@nikula.org> wrote: > > Hi Mark - > > > > I'm not sure which is worse, criticizing or rewriting other people's > > patches. I already did the former, and now I'm doing the > > latter. Apologies for both. I didn't really mean to write these patches, > > but it turned out to be more fun writing a proper reply in C than in > > English. > > > > Patch 1 adds --arg=true and --arg=false support for booleans. It's not > > strictly required for the --entire-thread support in patch 3, which uses > > the extension of keyword arguments from patch 2, but it's for > > consistency across boolean arguments. > > Hi > > I like patch 1: I have an almost identical to my version (in the series > I just sent to the list > id:"1331377533-30262-1-git-send-email-markwalters1009@gmail.com> > X-Mailer: git-send-email 1.7.9.1"). id:"1331377533-30262-1-git-send-email-markwalters1009@gmail.com" (fixed reference) Either of the patches should be pushed forward. The command line interface is sound, in line with notmuch style, and allows boolean parameters defaulting to true that you can switch off. And it needs no changes to argument parser users. > I am not sure about patch 2 and patch 3. Do you have a use case for > --option except when option is a boolean? For the sake of example, if some command produced some unformatted output by default, you could enable some formatting with --format and specify details with --format=foo or --format=bar. But admittedly it's a bit contrived. And using that for the bool case is also abusing it. > Otherwise I think I prefer either my > approach (abusing a notmuch_bool_t) or just adding an option > NOTMUCH_OPT_BOOLEAN_AS_INT which does boolean parsing but returns an > int. I guess I am saying that I think allowing boolean options which can > sometimes default to true and sometimes to false is more useful than > allowing --option for arbitrary keywords (*). > > What do you think? I think we're somewhere between overengineering and bikeshedding, and we should just fix the issue at hand with the "boolean" -1 value hack. Your v2 series accomplishes what's needed. Let's go with it, and consider a more general approach if another case comes up. (I actually wrote patches to add generic support for getting info about which arguments were set in the command line, but I think it's more trouble than its worth.) BR, Jani. > > Best wishes > > Mark > > (*) Indeed, I was thinking of the former as a possibility for the > exclude code, but I am erring towards just using keywords so I can allow > more options as you suggested. > > > > > > Please let me know what you think. > > > > BR, > > Jani. > > > > > > Jani Nikula (3): > > command-line-arguments: allow true and false keywords for booleans > > command-line-arguments: support keyword arguments with default value > > cli: allow switching off entire thread mode in notmuch show json > > format > > > > command-line-arguments.c | 45 +++++++++++++++++++++++++++++++++++++++------ > > command-line-arguments.h | 1 + > > notmuch-show.c | 12 ++++++++++-- > > 3 files changed, 50 insertions(+), 8 deletions(-) > > > > -- > > 1.7.5.4 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-03-11 19:26 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-08 22:15 [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN Mark Walters 2012-03-08 22:15 ` [PATCH 1/2] " Mark Walters 2012-03-08 22:15 ` [PATCH 2/2] cli: make --entire-thread option notmuch-show.c INT_OR_BOOLEAN Mark Walters 2012-03-09 0:48 ` [PATCH 0/2] cli: Parsing. Add option NOTMUCH_OPT_INT_OR_BOOLEAN Jani Nikula 2012-03-09 5:11 ` Mark Walters 2012-03-09 22:33 ` [PATCH 0/3] argument parsing additions Jani Nikula 2012-03-09 22:33 ` [PATCH 1/3] command-line-arguments: allow true and false keywords for booleans Jani Nikula 2012-03-09 22:33 ` [PATCH 2/3] command-line-arguments: support keyword arguments with default value Jani Nikula 2012-03-09 22:33 ` [PATCH 3/3] cli: allow switching off entire thread mode in notmuch show json format Jani Nikula 2012-03-10 11:23 ` [PATCH 0/3] argument parsing additions Mark Walters 2012-03-11 19:26 ` Jani Nikula
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).