Re: [PATCH 1/3] cli/show: convert keyword options to booleans

  • From: David Bremner <david@xxxxxxxxxxx>
  • To: Mark Walters <markwalters1009@xxxxxxxxx>, notmuch@xxxxxxxxxxxxx
  • Date: Tue, 09 May 2017 07:41:32 -0300

Mark Walters <markwalters1009@xxxxxxxxx> writes:

On Tue, 09 May 2017, David Bremner <david@xxxxxxxxxxx> wrote:
There are two keyword options here that impliment boolean options. It
is simpler to use the built-in boolean argument handling, and also
more robust against divergence in parsing boolean and keyword arguments.

Hi

Surely some case must go wrong for entire-thread? I think perhaps
notmuch-show --format=json --entire-thread=false <search-terms> will
still give the whole thread? (I can't easily test right now)

It seems not, at least in a quick test with your message.  It is true
that there seems to be no test of "show --format=json
--entire-thread=false" in T160-json.sh. So that's worth adding in any
case.

If I am reading the code right we are comparing the return value of the
boolean option (TRUE/FALSE) with the enum value for
ENTIRE_THREAD_DEFAULT/TRUE/FALSE.

Yeah, that's a bit icky, but we set ENTIRE_THREAD_FALSE = FALSE and
ENTIRE_THREAD_TRUE = TRUE at the start. Assuming this works at all, it
might be best to drop the enum.

Perhaps it would work if we reordered
the enum with ENTIRE_THREAD_DEFAULT last.

We force the values of all of the values of the enum, so order shouldn't
matter (or if it does matter they are already in ascending order).


Other related posts:

  1. Home
  2. notmuch
  3. Archive
  4. 05-2017