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