From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: Adam Wolfe Gordon <awg+notmuch@xvx.ca>
Subject: Re: [PATCH v4 2/4] reply: Add a JSON reply format.
In-Reply-To:
 <CAMoJFUshAUAO6hnYWefXgdBdZe3qavgWokLC8du3VWGYDb9RUQ@mail.gmail.com>
References: <1326995217-27423-1-git-send-email-awg+notmuch@xvx.ca>
	<1328746916-25447-1-git-send-email-awg+notmuch@xvx.ca>
	<1328746916-25447-3-git-send-email-awg+notmuch@xvx.ca>
	<87mx8sa2tj.fsf@gmail.com>
	<CAMoJFUshAUAO6hnYWefXgdBdZe3qavgWokLC8du3VWGYDb9RUQ@mail.gmail.com>
User-Agent: Notmuch/0.11.1+167~g6e72434 (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Fri, 10 Feb 2012 12:39:23 +0400
Message-ID: <8762ff9j5w.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Cc: notmuch@notmuchmail.org
Precedence: list

Hi Adam.

On Thu, 9 Feb 2012 21:27:29 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> w=
rote:
> Hi Dmitry,
>=20
> 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/
>=20
> Oops! git filter-branch to the rescue :-).
>=20
> >> Show is modified such that --format=3Djson no longer implies
> >> --entire-thread, as MUAs will use --format=3Djson when constructing
> >> replies. The man page is updated to reflect this change.
> >
> > I did not look into details. =C2=A0But I am surprised that user needs t=
o call
> > notmuch show --format=3Djson to make reply. =C2=A0I would expect notmuc=
h reply
> > to provide all required info (except for bodies).
>=20
> I agree, it would be ideal to include the data from show --format=3Djson
> 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.
>=20

I see.

> Will Austin's show rewrite make this easier/cleaner? Or am I being too
> squeamish about moving code?
>=20

It probably will.

> > Anyway, I think you should put this change in a separate patch.
>=20
> Yeah, if I leave it as is it changes a default behavior, so a separate
> patch would probably be a good idea.
>=20
> > Also, we clearly need a NEWS entry for it and user-customizable Emacs
> > variable changes. =C2=A0Though it can be done after this series is push=
ed, I
> > guess.
>=20
> 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.)
>=20

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.
>=20
> 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.
>=20

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