Re: [PATCH 0/9] argument parsing fixes and improvements

Subject: Re: [PATCH 0/9] argument parsing fixes and improvements

Date: Thu, 21 Sep 2017 20:07:30 +0300

To: Daniel Kahn Gillmor

Cc: Notmuch Mail

From: Jani Nikula


On Wed, 20 Sep 2017, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> right, about the defaults: as i'm sure you're aware, i'm trying to
> introduce some boolean options whose default is "unset", meaning "do
> whatever is set in the database config".  In particular, --try-decrypt
> (for those subcommands which do indexing).  As you can see in
> id:20170912230153.4175-8-dkg@fifthhorseman.net, i handle this by
> declaring the internal variable as:
>
>    int try_decrypt = -1;
>
> And then i decide to act on it with:
>
>    if (try_decrypt == TRUE || try_decrypt = FALSE) {
>       /* act on it… */
>    }
>
> Otherwise, i invoke the internal functions and let them behave as the
> database default does.

I think in general having stateful defaults are bad. See the code or
manual page for notmuch show. See how many defaults depend on some
*other* option, for absolutely no good reason. If you play with the
interface, you'll be surprised many times over how changing one knob
changes another as well. For example, switch output format, you'll get
entire threads as well. If you don't specify output format, choosing a
part will change the output format. Some unsupported combos result in
errors, some in warnings. It's a mess.

So I think I'd prefer either strict booleans where the default is one or
the other, or an explicit tristate true|false|default. In your case,
perhaps --try-decrypt=(true|false|database), where the default can be
requested and clearly documented. And that's again not a bool, like
notmuch search --exclude, but then neither is your proposed option. It
has three options in reality, but you've hidden one.

> This sort of goes against the trend of your
> id:5efab1e6d2250d87c909d9e5b435da5235efdc84.1505853159.git.jani@nikula.org,
> which prefers to use notmuch_bool_t for those declarations (and is maybe
> heading in the direction of stdbool.h).  How do you think we should
> implement a future boolean flag whose default value is "unset" if we
> head in this direction?

Indeed the direction is standard bools. I really see no reason to use
notmuch_bool_t except in the library interface for historical
reasons. Both sides of the interface could use stdbool.

I'm sure we could modify the argument parser to provide information
about options present and not, but given the above, I'm leaning towards
making the values explicit instead.

Further reading on orthogonality [1].

BR,
Jani.


[1] http://www.catb.org/esr/writings/taoup/html/ch04s02.html#orthogonality
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Thread: