e7/849dde01279fcb67e8948ebe13452e8d46b50a

   1 Return-Path: <dme@dme.org>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 3F16B429E35\r
   6         for <notmuch@notmuchmail.org>; Tue, 17 Jan 2012 01:04:51 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id 47NGKq4RDcL6 for <notmuch@notmuchmail.org>;\r
  16         Tue, 17 Jan 2012 01:04:47 -0800 (PST)\r
  17 Received: from mail-we0-f181.google.com (mail-we0-f181.google.com\r
  18         [74.125.82.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id A8D6E429E2F\r
  21         for <notmuch@notmuchmail.org>; Tue, 17 Jan 2012 01:04:47 -0800 (PST)\r
  22 Received: by werp13 with SMTP id p13so133006wer.26\r
  23         for <notmuch@notmuchmail.org>; Tue, 17 Jan 2012 01:04:46 -0800 (PST)\r
  24 Received: by 10.216.132.8 with SMTP id n8mr5298642wei.35.1326791086394;\r
  25         Tue, 17 Jan 2012 01:04:46 -0800 (PST)\r
  26 Received: from hotblack-desiato.hh.sledj.net\r
  27         (host81-149-164-25.in-addr.btopenworld.com. [81.149.164.25])\r
  28         by mx.google.com with ESMTPS id em13sm15537205wid.7.2012.01.17.01.04.44\r
  29         (version=TLSv1/SSLv3 cipher=OTHER);\r
  30         Tue, 17 Jan 2012 01:04:45 -0800 (PST)\r
  31 Received: by hotblack-desiato.hh.sledj.net (Postfix, from userid 30000)\r
  32         id ED75CA0048; Tue, 17 Jan 2012 09:04:42 +0000 (GMT)\r
  33 To: Adam Wolfe Gordon <awg+notmuch@xvx.ca>, notmuch@notmuchmail.org\r
  34 Subject: Re: [PATCH v2 4/4] emacs: Use the new JSON reply format.\r
  35 In-Reply-To: <1326737603-21166-5-git-send-email-awg+notmuch@xvx.ca>\r
  36 References: <1326737603-21166-1-git-send-email-awg+notmuch@xvx.ca>\r
  37         <1326737603-21166-5-git-send-email-awg+notmuch@xvx.ca>\r
  38 User-Agent: Notmuch/0.11+64~g42e8f66 (http://notmuchmail.org) Emacs/24.0.92.1\r
  39         (x86_64-pc-linux-gnu)\r
  40 From: David Edmondson <dme@dme.org>\r
  41 Date: Tue, 17 Jan 2012 09:04:39 +0000\r
  42 Message-ID: <cuny5t6r97c.fsf@hotblack-desiato.hh.sledj.net>\r
  43 MIME-Version: 1.0\r
  44 Content-Type: multipart/signed; boundary="=-=-=";\r
  45         micalg=pgp-sha1; protocol="application/pgp-signature"\r
  46 X-BeenThere: notmuch@notmuchmail.org\r
  47 X-Mailman-Version: 2.1.13\r
  48 Precedence: list\r
  49 List-Id: "Use and development of the notmuch mail system."\r
  50         <notmuch.notmuchmail.org>\r
  51 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  52         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  53 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  54 List-Post: <mailto:notmuch@notmuchmail.org>\r
  55 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  56 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  57         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  58 X-List-Received-Date: Tue, 17 Jan 2012 09:04:51 -0000\r
  59 \r
  60 --=-=-=\r
  61 Content-Type: text/plain\r
  62 Content-Transfer-Encoding: quoted-printable\r
  63 \r
  64 Much nicer now that it uses the mm stuff.\r
  65 \r
  66 On Mon, 16 Jan 2012 11:13:23 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> =\r
  67 wrote:\r
  68 > +(defun find-parts (parts type)\r
  69 \r
  70 Sorry for being a nuisance - this needs a name that indicates that it\r
  71 relates to notmuch. How about `notmuch-parts-filter-by-type'?\r
  72 \r
  73 > +  "Return a list of message parts with the given type"\r
  74 > +  (delq nil (mapcar (lambda (part)\r
  75 > +                   (if (string=3D (cdr (assq 'content-type part)) type)\r
  76 > +                       (cdr (assq 'content part))))\r
  77 > +                 parts)))\r
  78 \r
  79 '(delq nil ...)' can more readably be implemented using something like:\r
  80 \r
  81        (loop for part in parts\r
  82              if (string=3D (cdr (assq 'content-type part)) type)\r
  83              collect (cdr (assq 'content part)))\r
  84 \r
  85 (untested).\r
  86 \r
  87 > +(defun notmuch-mua-insert-part-quoted (part)\r
  88 > +  (save-restriction\r
  89 > +    (narrow-to-region (point) (point))\r
  90 > +    (insert part)\r
  91 > +    (goto-char (point-min))\r
  92 > +    (perform-replace "^" "> " nil t nil)\r
  93 \r
  94 Narrowing to '(point) (point)' seems a bit weird and using\r
  95 `perform-replace' is discouraged in programs. It would be more normal to\r
  96 use a loop of `re-search-forward' and `replace-match' with a limit after\r
  97 the insertion. Something like:\r
  98 \r
  99   (let ((start (point))\r
 100         limit)\r
 101     (insert part)\r
 102     (setq limit (point))\r
 103     (goto-char start)\r
 104     (while (re-search-forward "^" limit t)\r
 105       (replace-match "> "))\r
 106 \r
 107     ...\r
 108 \r
 109 (untested).\r
 110 \r
 111 > +    (insert "\n")\r
 112 > +    (set-buffer-modified-p nil)))\r
 113 =20=20=20=20=20=20\r
 114 Is this newline always required? Is it the cause of the spurious blank\r
 115 line down below?\r
 116 \r
 117 > +(defun notmuch-mua-parse-html-part (part)\r
 118 > +  (with-temp-buffer\r
 119 > +    (insert part)\r
 120 > +    (let ((handle (mm-make-handle (current-buffer) (list "text/html")))\r
 121 > +       (end-of-orig (point-max)))\r
 122 > +      (mm-display-part handle)\r
 123 > +      (kill-region (point-min) end-of-orig)\r
 124 > +      (fill-region (point-min) (point-max))\r
 125 > +      (buffer-substring (point-min) (point-max)))))\r
 126 \r
 127 `kill-region' will save content in the kill ring. Was that intended?\r
 128 (Maybe `delete-region' instead?)\r
 129 \r
 130 >  (defun notmuch-mua-reply (query-string &optional sender reply-all)\r
 131 > ...\r
 132 > +      (insert (format "On %s, %s wrote:\n"\r
 133 > +                   (cdr (assq 'date original-headers))\r
 134 > +                   (cdr (assq 'from original-headers))))\r
 135 \r
 136 I wonder whether emacs should be regenerating this or not. I'm okay with\r
 137 it, but previous discussion was that it should remain the responsibility\r
 138 of the CLI.\r
 139 \r
 140 > +      (if (null plain-parts)\r
 141 > +       (mapc (lambda (part) (notmuch-mua-insert-part-quoted (notmuch-mua-par=\r
 142 se-html-part part))) html-parts)\r
 143 > +     (mapc (lambda (part) (notmuch-mua-insert-part-quoted part)) plain-parts=\r
 144 ))\r
 145 \r
 146 Flip the 'then' and 'else' clauses to get rid of the `null'?\r
 147 \r
 148 > --- a/test/emacs\r
 149 > +++ b/test/emacs\r
 150 > @@ -270,6 +270,7 @@ Fcc: $(pwd)/mail/sent\r
 151 >  --text follows this line--\r
 152 >  On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmai=\r
 153 l.org> wrote:\r
 154 >  > This is a test that messages are sent via SMTP\r
 155 > +>=20\r
 156 \r
 157 It would be good if you could get rid of this trailing blank line.\r
 158 \r
 159 --=-=-=\r
 160 Content-Type: application/pgp-signature\r
 161 \r
 162 -----BEGIN PGP SIGNATURE-----\r
 163 Version: GnuPG v1.4.11 (GNU/Linux)\r
 164 \r
 165 iEYEARECAAYFAk8VOacACgkQaezQq/BJZRYbiQCfTfm/+mG4f3fm13Xii+EMUStQ\r
 166 HMgAn3Q8rXn1eqAhp2T731Yr+H5xHDND\r
 167 =GKQS\r
 168 -----END PGP SIGNATURE-----\r
 169 --=-=-=--\r