Re: [PATCH v3 2/5] reply: Add a JSON reply format.

From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: Adam Wolfe Gordon <awg+notmuch@xvx.ca>,
	Mark Walters <markwalters1009@gmail.com>
Cc: notmuch@notmuchmail.org
Subject: Re: [PATCH v3 2/5] reply: Add a JSON reply format.
Date: Sun, 05 Feb 2012 23:50:54 +0400	[thread overview]
Message-ID: <87aa4xys81.fsf@gmail.com> (raw)
In-Reply-To: <CAMoJFUusrzG9GcAwrbK0iNDaUOCVsc=e-i-n0tEcSOSF4XBn0Q@mail.gmail.com>

On Sun, 5 Feb 2012 12:42:12 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> Thanks for the review. The style nits are things I missed in my
> previous cleanup, so thanks for pointing them out. I should probably
> run uncrustify and see if it complains about anything else.
> 
> The other points are definitely up for discussion, and some are areas
> where I was unsure to start with. Discussion inline:
> 
> On Sun, Feb 5, 2012 at 04:50, Mark Walters <markwalters1009@gmail.com> wrote:
> >> +    /* We only care about inline text parts for reply purposes */
> >> +    if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE)) {
> >
> > This seems to be different from the logic in the text output: I think
> > that inlines all text/* regardless of disposition. I think the JSON
> > output should include at least as much as the text output as it is easy
> > for the caller to discard parts.
> 
> Indeed, the text output includes all text/* parts except for
> text/html, regardless of disposition. My thought was that it doesn't
> really make sense to quote an attachment, or at least it's not the
> behavior I would expect. But, perhaps it makes more sense to include
> all the text parts, with their dispositions, and let the MUA decide
> what it wants to quote. If anyone has thoughts on this I'm happy to
> hear them.
> 
> > Does wrapper need to a free/unref somewhere?
> 
> The text format doesn't free or unref wrapper, so I followed its
> example. But, I'm not a gmime expert, and I agree intuitively that it
> should be freed somehow. Can anyone enlighten me?
> 
> > If replying to multiple messages (such as a whole thread) you get
> > multiple sets of "new headers". I think that probably is not what is
> > wanted but its still better than the weird things the text version
> > does. Might be worth putting a comment. [What I think should happen is
> > that a union of all the headers from all these is taken throwing away
> > duplicate addresses but that is obviously not part of this patch set]
> 
> I've never been sure about what the intended behavior is when replying
> to multiple messages in the CLI. My thought was that it should create
> a reply to each message, so an MUA could iterate over them allowing
> you to compose replies to multiple messages. But, I've never wanted or
> used such a feature, so I'm agnostic on whether it's right. The emacs
> MUA (at least with my patch) ignores all but the first reply object in
> the array, my assumption being that reply only operates on multiple
> messages by accident.
> 

In some cases, notmuch CLI insists that the search query matches exactly
one message (e.g. notmuch show for parts).  IMO the same behavior makes
sense for notmuch reply.

Regards,
  Dmitry

> Does anyone use reply with multiple messages? If so, what semantics do
> you expect?
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch