MIME-Version: 1.0
Sender: awg@xvx.ca
In-Reply-To: <cuny5t6r97c.fsf@hotblack-desiato.hh.sledj.net>
References: <1326737603-21166-1-git-send-email-awg+notmuch@xvx.ca>
	<1326737603-21166-5-git-send-email-awg+notmuch@xvx.ca>
	<cuny5t6r97c.fsf@hotblack-desiato.hh.sledj.net>
Date: Tue, 17 Jan 2012 09:18:21 -0700
Message-ID:
 <CAMoJFUs9++xK15U3xpmKRGLR6pi_vWTq76KFywW2oksopegrew@mail.gmail.com>
Subject: Re: [PATCH v2 4/4] emacs: Use the new JSON reply format.
From: Adam Wolfe Gordon <awg+notmuch@xvx.ca>
To: David Edmondson <dme@dme.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Cc: notmuch@notmuchmail.org
Precedence: list

Hi David,

Thanks for the review. A couple of comments inline:

On Tue, Jan 17, 2012 at 02:04, David Edmondson <dme@dme.org> wrote:
>> + =A0 =A0(insert "\n")
>> + =A0 =A0(set-buffer-modified-p nil)))
>
> Is this newline always required? Is it the cause of the spurious blank
> line down below?

This is the cause of the spurious blank line, but without it the tests
complain about missing a blank line at the end of the file.  There may
be a better way to deal with this - I'll experiment.

>> =A0(defun notmuch-mua-reply (query-string &optional sender reply-all)
>> ...
>> + =A0 =A0 =A0(insert (format "On %s, %s wrote:\n"
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (cdr (assq 'date original-headers)=
)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (cdr (assq 'from original-headers)=
)))
>
> I wonder whether emacs should be regenerating this or not. I'm okay with
> it, but previous discussion was that it should remain the responsibility
> of the CLI.

I like this being generated in the MUA because then it can be
customized easily (e.g. in my later patch).  Of course, it would also
be possible to add this as a config option for the CLI and generate it
there, but it feels to me like if there's a line between notmuch and
the MUA, this belongs on the MUA side, especially in MUAs that are
formatting the reply themselves anyway.

I'm happy to hear more discussion on this, and will implement whatever
seems best.  I don't actually use the customization myself, it just
strikes me as a useful feature.