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