Re: [PATCH v2 0/2] cli: Allow true/false parameter for boolean

Subject: Re: [PATCH v2 0/2] cli: Allow true/false parameter for boolean

Date: Tue, 13 Mar 2012 22:47:16 -0400

To: Mark Walters

Cc: notmuch@notmuchmail.org

From: Austin Clements


Quoth Mark Walters on Mar 10 at 11:05 am:
> Hi
> 
> Here is a second version of a patch to allow parameters to boolean
> options on the command line. This version allows parameters
> (=true|false). My first version is at
> id:"1331244944-7960-1-git-send-email-markwalters1009@gmail.com". Jani
> posted an alternative version there. Jani's version and this one are
> quite similar: the key difference is that this version abuses a
> notmuch_bool_t by setting it to -1 (to indicate that the parser has
> not set this option). This makes the code simpler but is definitely an
> abuse. I will discuss this further in replies to Jani's series.
> 
> Best wishes
> 
> Mark
> 
> Mark Walters (2):
>   cli: Parsing. Allow true/false parameter for boolean options.
>   cli: make --entire-thread=false work for format=json.

LGTM.  The one question I have is whether or not the resulting
non-entire-thread behavior of the JSON format is actually what we
*want*.  As a probably unintentional consequence of the current show
code, we get

# A message and its replies (show_messages)
thread_node = [
    message?,                 # present if --entire-thread or matched
    [thread_node*]            # children of message
]

But would it be better to have

thread_node = [
    message|null,             # non-null if --entire-thread or matched
    [thread_node*]            # children of message
]

?  The latter is much more natural for consumers to work with
(checking whether the message matched or not is more natural and the
index of the child list doesn't change), but would require a little
more code in show to support.

Thread: