Hi Adam. On Thu, 9 Feb 2012 21:27:29 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote: > Hi Dmitry, > > On Thu, Feb 9, 2012 at 00:22, Dmitry Kurochkin > <dmitry.kurochkin@gmail.com> wrote: > >> Reply now enforces that only one message is returned, as the semantics > >> of replying to multiple messages are not wel-defined. > > > > s/wel/well/ > > Oops! git filter-branch to the rescue :-). > > >> Show is modified such that --format=json no longer implies > >> --entire-thread, as MUAs will use --format=json when constructing > >> replies. The man page is updated to reflect this change. > > > > I did not look into details. But I am surprised that user needs to call > > notmuch show --format=json to make reply. I would expect notmuch reply > > to provide all required info (except for bodies). > > I agree, it would be ideal to include the data from show --format=json > in the reply JSON. I started down the path of implementing this, but > realized it requires either copying quite a bit of code from show or > factoring it out, and both options felt kind of dirty. I'd like show > and reply to share a function that produces the JSON-formatted body of > a message, but it doesn't feel right to expose the entire JSON format > and all the functions that go with it from notmuch-show.c and put the > structure and all the prototypes in notmuch-client.h. > I see. > Will Austin's show rewrite make this easier/cleaner? Or am I being too > squeamish about moving code? > It probably will. > > Anyway, I think you should put this change in a separate patch. > > Yeah, if I leave it as is it changes a default behavior, so a separate > patch would probably be a good idea. > > > Also, we clearly need a NEWS entry for it and user-customizable Emacs > > variable changes. Though it can be done after this series is pushed, I > > guess. > > I don't think there are any emacs customization changes here, unless > we want to implement the notmuch mode config file that was discussed > before to give message-citation-line-format and other things nice > defaults. Or are you suggesting that there should be some new > customization options? (One I can think of would be the list of > preferred types for multipart/alternative display, which right now is > hardcoded in notmuch-show.el.) > No. I thought there were some user-customizable variables that were moved to notmuch-lib and renamed. If that is correct, it would not hurt to document these changes. > > Thank you for this work, it is much appreciated. > > Thanks for taking the time to review these patches! I think as a > result of everyone's reviews I've pushed the series toward what I > envisioned/wanted in the first place, rather than the kinda kludgey > thing I did initially. > No need to thank me for review, I did not do it :) > Amusingly, I've started a new job since I wrote the original patch and > no longer receive much HTML-only email, but I like this patch series > enough to see it through anyway :-). Congratulations with the new job :) Regards, Dmitry