On Sat, 26 May 2012, Peter Wang <novalazy at gmail.com> wrote:
> On Tue, 24 Apr 2012 10:11:13 +0100, Mark Walters <markwalters1009 at
> 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
