Re: [PATCH 1/2] cli: make --entire-thread=false work for format=json.

[Mark Walters <markwalters1009@gmail.com>]

On Tue, 10 Apr 2012, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> Hi Mark,
>
> This looks good to me, but I haven't tested it. It's probably worth
> adding a test for the new functionality.

Yes I agree. I could add a test to the json tests, or I could update the
pair of test patches at
id:"1332171061-27983-1-git-send-email-markwalters1009@gmail.com" as
there do not seem to be any tests for --entire-thread or not for the
other formats either. What do you think?

> One style issue below, which is a matter of taste and I'll defer to
> others if they disagree:
>
> On Tue, Apr 10, 2012 at 11:04, Mark Walters <markwalters1009@gmail.com> wrote:
>> @@ -895,10 +905,11 @@ show_messages (void *ctx,
>>            if (status && !res)
>>                res = status;
>>            next_indent = indent + 1;
>> +       } else
>> +           status = show_null_message (format);
>
> I accept, but don't particularly like, the notmuch style of omitting
> braces where they aren't required. However, an else with a brace on
> only one side just looks weird. If they're like this everywhere else
> then I guess it's best to be consistent, but to me it's a lot more
> readable as } else {.
>
> As I said above, I'll defer to others' judgement here, just thought it
> was worth pointing out.

Yes I will fix this in the next version (I recall Jani agreed with you
so I will take that as preferred unless someone says otherwise.)

thanks

Mark