Re: [PATCH 4/4] show: Rewrite show_message_body to use the MIME tree interface.

2011-12-23 Thread Austin Clements
Quoth Dmitry Kurochkin on Dec 11 at  2:34 pm:
 Hi Austin.
 
 I enjoyed reviewing this patch.  It is a pleasure to see how complex and
 confusing code becomes much smaller and cleaner.
 
 I still have some questions with the new code.  It seems confusing to me
 that part_content is called first and then go envelope headers.  But I
 this is just the first step of the rewrite, right? :)

Yeah, this is weird, but I'm just being compatible with the existing
code at this point.  This code is about to go away.

 The only comment I have:
 
 +format-part_content (part);
 
 For all other format members that are function pointers, we have a check
 for NULL.  Perhaps we should add it here as well?

I would if I weren't about to delete this.  ]:--8)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 4/4] show: Rewrite show_message_body to use the MIME tree interface.

2011-12-11 Thread Dmitry Kurochkin
Hi Austin.

I enjoyed reviewing this patch.  It is a pleasure to see how complex and
confusing code becomes much smaller and cleaner.

I still have some questions with the new code.  It seems confusing to me
that part_content is called first and then go envelope headers.  But I
this is just the first step of the rewrite, right? :)

The only comment I have:

+format-part_content (part);

For all other format members that are function pointers, we have a check
for NULL.  Perhaps we should add it here as well?

Regards,
  Dmitry
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 4/4] show: Rewrite show_message_body to use the MIME tree interface.

2011-11-29 Thread Jani Nikula
On Sun, 27 Nov 2011 21:21:11 -0500, Austin Clements amdra...@mit.edu wrote:
 Since this is a rewrite, the diff is not very enlightening.  It's
 easier to look at the old code and the new code side-by-side.

Hi Austin, try the git format-patch --break-rewrites option. It works
nicely on patches like this. See below.

BR,
Jani.


From 4df3de0d35b7fb5b142b3413d56cda2811406fd9 Mon Sep 17 00:00:00 2001
From: Austin Clements amdra...@mit.edu
Date: Sun, 27 Nov 2011 21:21:11 -0500
Subject: [PATCH] show: Rewrite show_message_body to use the MIME tree
 interface.

This removes all of the MIME traversal logic from show_message_body
and leaves only its interaction with the format callbacks.

Besides isolating concerns, since traversal happens behind a trivial
interface, there is now much less code duplication in
show_message_part.  Also, this uses mime_node_seek_dfs to start at the
requested part, eliminating all of the logic about parts being
selected or being in_zone (and reducing the show state to only a
part counter).  notmuch_show_params_t no longer needs to be passed
through the recursion because the only two fields that mattered
(related to crypto) are now handled by the MIME tree.

The few remaining complexities in show_message_part highlight
irregularities in the format callbacks with respect to top-level
messages and embedded message parts.

Since this is a rewrite, the diff is not very enlightening.  It's
easier to look at the old code and the new code side-by-side.
---
 show-message.c |  329 +--
 1 files changed, 102 insertions(+), 227 deletions(-)
 rewrite show-message.c (81%)

diff --git a/show-message.c b/show-message.c
dissimilarity index 81%
index 09fa607..4560aea 100644
--- a/show-message.c
+++ b/show-message.c
@@ -1,227 +1,102 @@
-/* notmuch - Not much of an email program, (just index and search)
- *
- * Copyright © 2009 Carl Worth
- * Copyright © 2009 Keith Packard
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see http://www.gnu.org/licenses/ .
- *
- * Authors: Carl Worth cwo...@cworth.org
- * Keith Packard kei...@keithp.com
- */
-
-#include notmuch-client.h
-
-typedef struct show_message_state {
-int part_count;
-int in_zone;
-} show_message_state_t;
-
-static void
-show_message_part (GMimeObject *part,
-  show_message_state_t *state,
-  const notmuch_show_format_t *format,
-  notmuch_show_params_t *params,
-  int first)
-{
-GMimeObject *decryptedpart = NULL;
-int selected;
-state-part_count += 1;
-
-if (! (GMIME_IS_PART (part) || GMIME_IS_MULTIPART (part) || 
GMIME_IS_MESSAGE_PART (part))) {
-   fprintf (stderr, Warning: Not displaying unknown mime part: %s.\n,
-g_type_name (G_OBJECT_TYPE (part)));
-   return;
-}
-
-selected = (params-part = 0 || state-part_count == params-part);
-
-if (selected || state-in_zone) {
-   if (!first  (params-part = 0 || state-in_zone))
-   fputs (format-part_sep, stdout);
-
-   if (format-part_start)
-   format-part_start (part, (state-part_count));
-}
-
-/* handle PGP/MIME parts */
-if (GMIME_IS_MULTIPART (part)  params-cryptoctx) {
-   GMimeMultipart *multipart = GMIME_MULTIPART (part);
-   GError* err = NULL;
-
-   if (GMIME_IS_MULTIPART_ENCRYPTED (part)  params-decrypt)
-   {
-   if ( g_mime_multipart_get_count (multipart) != 2 ) {
-   /* this violates RFC 3156 section 4, so we won't bother with 
it. */
-   fprintf (stderr,
-Error: %d part(s) for a multipart/encrypted message 
(should be exactly 2)\n,
-g_mime_multipart_get_count (multipart));
-   } else {
-   GMimeMultipartEncrypted *encrypteddata = 
GMIME_MULTIPART_ENCRYPTED (part);
-   decryptedpart = g_mime_multipart_encrypted_decrypt 
(encrypteddata, params-cryptoctx, err);
-   if (decryptedpart) {
-   if ((selected || state-in_zone)  format-part_encstatus)
-   format-part_encstatus (1);
-   const GMimeSignatureValidity *sigvalidity = 
g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
-   if (!sigvalidity)
-   fprintf (stderr, Failed to verify signed part: %s\n, 
(err ? err-message