Date: Fri, 17 Feb 2012 22:30:43 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: Adam Wolfe Gordon <awg+notmuch@xvx.ca>
Subject: Re: [PATCH v5.2 7/7] emacs: Use the new JSON reply format and
	message-cite-original
Message-ID: <20120218033043.GJ5991@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>
	<CAMoJFUs5jnu7=5PtrNib9J6P_FnxD8bK2L+oQiBdgXPJXhvMJg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To:
 <CAMoJFUs5jnu7=5PtrNib9J6P_FnxD8bK2L+oQiBdgXPJXhvMJg@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Adam Wolfe Gordon on Feb 17 at  7:22 pm:
> 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.  Besides 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.
> 
> >> +     (goto-char (point-max)))
> >> +
> >> +      (let ((from (cdr (assq 'From original-headers)))
> >> +         (date (cdr (assq 'Date original-headers)))
> >> +         (start (point)))
> >> +
> >> +     (insert "From: " from "\n")
> >> +     (insert "Date: " date "\n\n")
> >
> > Sorry; I'm having trouble following the diff.  What 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.

Ah, okay.  Is this how it generates the citation line?

Could definitely do with a comment.

> >> +     (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.
> 
> >> +     (goto-char start)
> >> +     ;; Quote the original message according to the user's configured style.
> >> +     (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.

We should probably stick with the original behavior, at least for now.

I just noticed that message-cite-original calls mml-quote-region.  How
is it that we don't wind up double-quoting MML tags with this change?