From: Mark Walters <markwalters1009@gmail.com>
To: Peter Wang <novalazy@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v6 3/6] cli: make --entire-thread=false work for
	format=json.
In-Reply-To: <20120528233415.GE2331@hili.localdomain>
References: <1338106946-7611-1-git-send-email-markwalters1009@gmail.com>
	<1338106946-7611-4-git-send-email-markwalters1009@gmail.com>
	<20120528233415.GE2331@hili.localdomain>
User-Agent: Notmuch/0.13+55~g992aa73 (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Mon, 28 May 2012 20:06:38 +0100
Message-ID: <87txz0m8g1.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Mon, 28 May 2012, Peter Wang <novalazy@gmail.com> wrote:
> On Sun, 27 May 2012 09:22:23 +0100, Mark Walters <markwalters1009@gmail.com> wrote:
>> @@ -1036,7 +1047,9 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>>      switch (format_sel) {
>>      case NOTMUCH_FORMAT_JSON:
>>  	format = &format_json;
>> -	params.entire_thread = TRUE;
>> +	/* JSON defaults to entire-thread TRUE */
>> +	if (entire_thread == ENTIRE_THREAD_DEFAULT)
>> +	    entire_thread = ENTIRE_THREAD_TRUE;
>>  	break;
>
> Minor point, but you can defer this until later and keep the logic in
> one place.
>
>>      case NOTMUCH_FORMAT_TEXT:
>>  	format = &format_text;
>> @@ -1058,6 +1071,15 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>>  	params.raw = TRUE;
>>  	break;
>>      }
>> +    /* Default is entire-thread = FALSE except for format=json which
>> +     * is dealt with above. */
>> +    if (entire_thread == ENTIRE_THREAD_DEFAULT)
>> +	entire_thread = ENTIRE_THREAD_FALSE;
>> +
>> +    if (entire_thread == ENTIRE_THREAD_TRUE)
>> +	params.entire_thread = TRUE;
>> +    else
>> +	params.entire_thread = FALSE;
>
>     /* Default is entire-thread = FALSE except for format=json. */
>     if (entire_thread == ENTIRE_THREAD_DEFAULT) {
> 	if (format == &format_json)
> 	    entire_thread = ENTIRE_THREAD_TRUE;
> 	else
> 	    entire_thread = ENTIRE_THREAD_FALSE;
>     }

Yes I agree that this is cleaner. I will put it in the next version.

Many thanks for the reviews!

Mark