From 278fe2b384d97eef48d95ddaf2c018dfb829a8d8 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Sat, 18 Feb 2012 22:23:52 +1900 Subject: [PATCH] Re: [PATCH v5.2 3/7] reply: Add a JSON reply format. --- 21/6838140433901ffc5b375a6fed8678ceeea6bb | 152 ++++++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 21/6838140433901ffc5b375a6fed8678ceeea6bb diff --git a/21/6838140433901ffc5b375a6fed8678ceeea6bb b/21/6838140433901ffc5b375a6fed8678ceeea6bb new file mode 100644 index 000000000..af309501a --- /dev/null +++ b/21/6838140433901ffc5b375a6fed8678ceeea6bb @@ -0,0 +1,152 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 2BE95431FB6 + for ; Fri, 17 Feb 2012 19:25:43 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id R4oOjfJOOD7l for ; + Fri, 17 Feb 2012 19:25:42 -0800 (PST) +Received: from dmz-mailsec-scanner-8.mit.edu (DMZ-MAILSEC-SCANNER-8.MIT.EDU + [18.7.68.37]) + by olra.theworths.org (Postfix) with ESMTP id 7F3CF431FAE + for ; Fri, 17 Feb 2012 19:25:42 -0800 (PST) +X-AuditID: 12074425-b7f4a6d0000008e0-c5-4f3f1a34da45 +Received: from mailhub-auth-2.mit.edu ( [18.7.62.36]) + by dmz-mailsec-scanner-8.mit.edu (Symantec Messaging Gateway) with SMTP + id 9A.11.02272.43A1F3F4; Fri, 17 Feb 2012 22:25:40 -0500 (EST) +Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) + by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id q1I3PdCr014641; + Fri, 17 Feb 2012 22:25:39 -0500 +Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) + (authenticated bits=0) + (User authenticated as amdragon@ATHENA.MIT.EDU) + by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q1I3PbY0017290 + (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); + Fri, 17 Feb 2012 22:25:38 -0500 (EST) +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77) + (envelope-from ) + id 1Ryats-0004Bi-3T; Fri, 17 Feb 2012 22:23:52 -0500 +Date: Fri, 17 Feb 2012 22:23:52 -0500 +From: Austin Clements +To: Adam Wolfe Gordon +Subject: Re: [PATCH v5.2 3/7] reply: Add a JSON reply format. +Message-ID: <20120218032352.GI5991@mit.edu> +References: <1329361957-28493-1-git-send-email-awg+notmuch@xvx.ca> + <1329361957-28493-4-git-send-email-awg+notmuch@xvx.ca> + <20120217170457.GE5991@mit.edu> + +MIME-Version: 1.0 +Content-Type: text/plain; charset=iso-8859-1 +Content-Disposition: inline +Content-Transfer-Encoding: 8bit +In-Reply-To: + +User-Agent: Mutt/1.5.21 (2010-09-15) +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFprDKsWRmVeSWpSXmKPExsUixG6nomsiZe9v8PIXp8WRPbPYLa7fnMns + wOTxbNUtZo+mH4tZA5iiuGxSUnMyy1KL9O0SuDI+zW5hLOiVqZh79Th7A+MBsS5GTg4JAROJ + Q4fXM0PYYhIX7q1n62Lk4hAS2McosfPFJXYIZwOjxOUJ/6AyJ5kktj2fwQLhLGGUuH2xB8jh + 4GARUJW4eNcRZBSbgIbEtv3LGUFsEQEtiR/rv7KC2MwC0hLffjczgdjCArYSzz5dAlvNK6At + cerqDGaImU8YJU6cWMsCkRCUODnzCQtEs47Ezq132EB2gQxa/o8DIiwv0bx1NtgcToFAib2H + 1oLNFxVQkZhychvbBEbhWUgmzUIyaRbCpFlIJi1gZFnFKJuSW6Wbm5iZU5yarFucnJiXl1qk + a6GXm1mil5pSuokRHAkuqjsYJxxSOsQowMGoxMP7qtPOX4g1say4MvcQoyQHk5Io7xkxe38h + vqT8lMqMxOKM+KLSnNTiQ4wSHMxKIrxed4DKeVMSK6tSi/JhUtIcLErivJpa7/yEBNITS1Kz + U1MLUotgsjIcHEoSvNmSQEMFi1LTUyvSMnNKENJMHJwgw3mAhneB1PAWFyTmFmemQ+RPMSpK + ifPmgyQEQBIZpXlwvbBE9YpRHOgVYd5WkCoeYJKD634FNJgJaDCvEMjVxSWJCCmpBsZdR6cI + m81cGXXG9TxXQ8+jRZZTdFvaGvWknuw5+0Bjn4im1P4dx+/Gpbi1e+yX3y/84rHGuTLhuMJ5 + bGvLlZ88ubvoTut+2cZrO3I3XnW7YDvF/9i7KlsmU/GQwu+OMbsn9a9qaWXh3Wx4RrLKIGn6 + Bu6wy2Iq5bpr599c/aJsLqON0EWN35JKLMUZiYZazEXFiQA0LyOPLwMAAA== +Cc: notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Sat, 18 Feb 2012 03:25:43 -0000 + +Quoth Adam Wolfe Gordon on Feb 17 at 7:06 pm: +> On Fri, Feb 17, 2012 at 10:04, Austin Clements wrote: +> > The first two patches LGTM.  A few nits in this one. +> +> Thanks for the review. A couple of points to discuss below; everything +> else I'll change for the next version. +> +> >> +void +> >> +format_part_json (const void *ctx, mime_node_t *node, notmuch_bool_t first); +> >> + +> > +> > This is the wrong place for this declaration, since it is not part of +> > the MIME node abstraction.  It should go somewhere above the /* +> > notmuch-config.c */ comment.  Above that, it's a bit of a jumble.  I'd +> > probably put it right after notmuch_show_command. +> +> Agreed. I initially had it earlier in the file (with the other +> show-related functions), but moved it down since it requires the +> mime_node_t declaration. There are a couple of options: put in a +> pre-declaration of mime_node_t early in the file, or move the +> mime_node stuff to a separate header file and include it in +> notmuch-client.h. I lean toward the latter, since notmuch-client.h is +> getting very big as it is. Thoughts? + +struct mime_node is already declared at the top of notmuch-client.h. +That should probably just be replaced with + typedef struct mime_node mime_node_t; +(and notmuch_show_format.part can be updated to take a mime_node_t *). +Alternatively, you could change your declaration to take a struct +mime_node, but it's nicer if the declaration and the definition match +literally and not just logically. Moving mime_node_t into its own +header isn't a bad idea on its own (in fact, I specifically wrote it +so it could live in util/ if we wanted), but seems like overkill for +this. + +> >> +    if (notmuch_query_count_messages (query) != 1) { +> >> +     fprintf (stderr, "Error: search term did not match precisely one message.\n"); +> >> +     return 1; +> >> +    } +> > +> > Technically count_messages does not have to be accurate, but since +> > this is the same thing notmuch-show does, it's probably fine for now. +> +> Ah, I didn't realize this. I just followed the show example. + +I think it's fine as is. Probably as a later, independent patch, we +should update both places to just check the iterator after the call to +notmuch_messages_get. Or you could update it as an extra minipatch in +your series if you want. + +> > Perhaps we should add proper handling of multi-message replies to +> > devel/TODO? +> +> Probably a good idea, although it means defining what proper handling +> of multi-message replies in the CLI means. Personally, I don't think +> it makes much sense to reply to multiple messages. The only place that +> functionality is actually used (AFAIK) is in notmuch-search.el, which, +> with my patches, throws an error if you try to reply to a thread +> containing multiple messages. In my mind, the correct behavior in that +> specific case is to create a reply to the last message in the thread, +> which is better handled in the emacs code than the CLI anyway. + +*Doing* it requires defining what it means, but devel/TODO is a fine +place for arbitrarily fantastical and under-specified desires. That +said, I don't think defining it is that hard. We could just do +whatever mutt does. The body of a multi-message reply in mutt is the +concatenation of the bodies that would be generated for individual +replies and I suspect the headers are the gathered up to/cc addresses, +an in-reply-to that lists all of the replied to message IDs, and a +subject and references header derived from the first message replied +to. -- 2.26.2