unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: Austin Clements <amdragon@MIT.EDU>
To: Adam Wolfe Gordon <awg+notmuch@xvx.ca>
Cc: notmuch@notmuchmail.org
Subject: Re: [PATCH v5.2 7/7] emacs: Use the new JSON reply format and message-cite-original
Date: Fri, 17 Feb 2012 22:30:43 -0500	[thread overview]
Message-ID: <20120218033043.GJ5991@mit.edu> (raw)
In-Reply-To: <CAMoJFUs5jnu7=5PtrNib9J6P_FnxD8bK2L+oQiBdgXPJXhvMJg@mail.gmail.com>

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?

  reply	other threads:[~2012-02-18  3:32 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-16  3:12 [PATCH v5.2 0/7] Reply enhancements, tidied up Adam Wolfe Gordon
2012-02-16  3:12 ` [PATCH v5.2 1/7] test: Add broken test for the new JSON reply format Adam Wolfe Gordon
2012-02-17 18:20   ` Austin Clements
2012-02-16  3:12 ` [PATCH v5.2 2/7] reply: Factor out reply creation Adam Wolfe Gordon
2012-02-16  3:12 ` [PATCH v5.2 3/7] reply: Add a JSON reply format Adam Wolfe Gordon
2012-02-17 17:04   ` Austin Clements
2012-02-18  2:06     ` Adam Wolfe Gordon
2012-02-18  3:23       ` Austin Clements
2012-02-16  3:12 ` [PATCH v5.2 4/7] man: Update notmuch-reply man page for JSON format Adam Wolfe Gordon
2012-02-17 20:01   ` Austin Clements
2012-02-16  3:12 ` [PATCH v5.2 5/7] emacs: Factor out useful functions into notmuch-lib Adam Wolfe Gordon
2012-02-16  3:12 ` [PATCH v5.2 6/7] test: Add broken tests for new emacs reply functionality Adam Wolfe Gordon
2012-02-16  3:12 ` [PATCH v5.2 7/7] emacs: Use the new JSON reply format and message-cite-original Adam Wolfe Gordon
2012-02-17 20:00   ` Austin Clements
2012-02-18  2:22     ` Adam Wolfe Gordon
2012-02-18  3:30       ` Austin Clements [this message]
2012-02-22  6:46     ` Adam Wolfe Gordon
2012-02-21  5:59   ` Austin Clements
2012-02-21 16:49     ` Adam Wolfe Gordon
2012-02-22 16:57       ` Austin Clements

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://notmuchmail.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120218033043.GJ5991@mit.edu \
    --to=amdragon@mit.edu \
    --cc=awg+notmuch@xvx.ca \
    --cc=notmuch@notmuchmail.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).