Received-SPF: pass (google.com: domain of awg@xvx.ca designates 10.152.123.68
	as permitted sender) client-ip=10.152.123.68; 
MIME-Version: 1.0
Sender: awg@xvx.ca
In-Reply-To: <20120217200017.GG5991@mit.edu>
References: <1329361957-28493-1-git-send-email-awg+notmuch@xvx.ca>
	<1329361957-28493-8-git-send-email-awg+notmuch@xvx.ca>
	<20120217200017.GG5991@mit.edu>
Date: Fri, 17 Feb 2012 19:22:07 -0700
Message-ID:
 <CAMoJFUs5jnu7=5PtrNib9J6P_FnxD8bK2L+oQiBdgXPJXhvMJg@mail.gmail.com>
Subject: Re: [PATCH v5.2 7/7] emacs: Use the new JSON reply format and
	message-cite-original
From: Adam Wolfe Gordon <awg+notmuch@xvx.ca>
To: Austin Clements <amdragon@mit.edu>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Cc: notmuch@notmuchmail.org
Precedence: list

On Fri, Feb 17, 2012 at 13:00, Austin Clements <amdragon@mit.edu> wrote:
> One general comment that affects a lot of things in this patch: I
> think you should use the same JSON parsing settings that
> notmuch-query-get-threads uses. =A0Besides consistency and more
> opportunities for code reuse, using lists instead of vectors for JSON
> arrays will simplify a lot of this code without any drawbacks.

I pretty much agree. The only reason I stuck with alists was, as you
mention below, to be compatible with certain mail functions. Given the
things you've pointed out, I think the small hassle of making those
work with plists is worthwhile, so I'll give it a go.

Clarification on a couple of things follow, otherwise I'll make all
these changes for the next version.

>> + =A0 =A0 (goto-char (point-max)))
>> +
>> + =A0 =A0 =A0(let ((from (cdr (assq 'From original-headers)))
>> + =A0 =A0 =A0 =A0 (date (cdr (assq 'Date original-headers)))
>> + =A0 =A0 =A0 =A0 (start (point)))
>> +
>> + =A0 =A0 (insert "From: " from "\n")
>> + =A0 =A0 (insert "Date: " date "\n\n")
>
> Sorry; I'm having trouble following the diff. =A0What are the inserts
> for?

The function message-cite-original cites an original message, which is
in the marked region. It assumes the headers of the original message
will be part of the marked region, but the only ones it actually uses
are From and Date.

This could probably use a comment in the code.

>> + =A0 =A0 (push-mark)
>
> It's unfortunate that message-cite-original depends on the mark.
> Since you're about to push the mark for the user anyway, maybe this
> should be a set-mark so that only one mark gets pushed?

Probably the right thing to do.

>> + =A0 =A0 (goto-char start)
>> + =A0 =A0 ;; Quote the original message according to the user's configur=
ed style.
>> + =A0 =A0 (message-cite-original))))
>
> message-cite-original-without-signature?

Perhaps it should be configurable (notmuch-reply-cite-function or
somesuch). I believe message-cite-original matches the behavior of the
old reply, which didn't strip signatures, but I don't have strong
feelings either way.

>> + =A0(push-mark)
>
> Is message-cite-original guaranteed to leave point in a reasonable
> place for this or should we create our own marker above (probably
> after the if re-search-backward..) and use it here to get point to the
> right place?

In my experience, it leaves the point at the end of the quoted region,
but the documentation doesn't make any guarantees. Probably safer to
set a marker.