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

[Daniel Kahn Gillmor <dkg@fifthhorseman.net>]

[-- Attachment #1: Type: text/plain, Size: 3049 bytes --]

Hi Jani--

On Wed 2017-09-20 12:16:33 +0300, Jani Nikula wrote:
> Those are good questions, and up for debate. I was more interested in
> getting the mask out behaviour for keyword flag arguments, and the
> boolean part came practically free. Or, there was certain synergy in
> having them behave similarly.

gotcha, that motivation makes sense, thanks!

> Now, I don't know if that's the best way to do this. Perhaps people
> would prefer --output=!entire-thread or --output=no-entire-thread or
> --output=entire-thread-disable or something. But the --no- prefix at
> least has precedence in other software for booleans, and I think
> that's a good reason to consider it. And having patches on the list
> doing it one way generates better feedback and discussion than just
> general musing on IRC.

yup.  fwiw, if we want negations like this, i'm fine with the --no-
prefix as you've proposed.

>> I'm a little concerned that "notmuch search --exclude" is a corner case
>> that doesn't make a lot of sense here.
>
> That was an unfortunate choice for an example. notmuch show --exclude
> is a normal boolean option I was looking at. The search --exclude
> option shouldn't be discussed as an example for anything. ;)

*you* didn't use "notmuch search --exclude" -- i was the one who brought
it up because i've been looking at argument parsing recently and it
stuck out as a bit of a weirdo.  It's too bad that it doesn't fit into
this framework, but meh -- a foolish consistency is the hobgoblin of
small minds.

> Of course. But given that there's so much bikeshedding potential, I
> decided to postpone that part. :) We also have mixed documentation of
> boolean options as it is. Some just have --foo, others have
> --foo=(true|false). Perhaps we use the former for default false
> options, and the latter for default true options, but it's still a bit
> inconsistent. With --no-foo support, we could document the default
> true options with that, and it would convey the default as a bonus.

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.

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?

          --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]