c1/e60daf79134afdbce66971976cd3731c953634

   1 Return-Path: <amdragon@mit.edu>\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 5A437431FAF\r
   6         for <notmuch@notmuchmail.org>; Thu, 24 Apr 2014 11:12:30 -0700 (PDT)\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 JmVCETnxE4pR for <notmuch@notmuchmail.org>;\r
  16         Thu, 24 Apr 2014 11:12:24 -0700 (PDT)\r
  17 Received: from dmz-mailsec-scanner-5.mit.edu (dmz-mailsec-scanner-5.mit.edu\r
  18         [18.7.68.34])\r
  19         by olra.theworths.org (Postfix) with ESMTP id 94479431FAE\r
  20         for <notmuch@notmuchmail.org>; Thu, 24 Apr 2014 11:12:24 -0700 (PDT)\r
  21 X-AuditID: 12074422-f79186d00000135a-cd-53595407e39e\r
  22 Received: from mailhub-auth-1.mit.edu ( [18.9.21.35])\r
  23         (using TLS with cipher AES256-SHA (256/256 bits))\r
  24         (Client did not present a certificate)\r
  25         by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP\r
  26         id 4B.79.04954.70459535; Thu, 24 Apr 2014 14:12:23 -0400 (EDT)\r
  27 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
  28         by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id s3OICM0Z023927; \r
  29         Thu, 24 Apr 2014 14:12:23 -0400\r
  30 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  31         (authenticated bits=0)\r
  32         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  33         by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id s3OICKge007507\r
  34         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  35         Thu, 24 Apr 2014 14:12:21 -0400\r
  36 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  37         (envelope-from <amdragon@MIT.EDU>)\r
  38         id 1WdO8B-0005Qt-Kd; Thu, 24 Apr 2014 14:12:19 -0400\r
  39 Date: Thu, 24 Apr 2014 14:12:17 -0400\r
  40 From: Austin Clements <amdragon@MIT.EDU>\r
  41 To: Mark Walters <markwalters1009@gmail.com>\r
  42 Subject: Re: [PATCH 08/11] emacs: Support caching in\r
  43         notmuch-get-bodypart-{binary, text}\r
  44 Message-ID: <20140424181216.GO25817@mit.edu>\r
  45 References: <1398105468-14317-1-git-send-email-amdragon@mit.edu>\r
  46         <1398105468-14317-9-git-send-email-amdragon@mit.edu>\r
  47         <87sip3t37f.fsf@qmul.ac.uk>\r
  48 MIME-Version: 1.0\r
  49 Content-Type: text/plain; charset=us-ascii\r
  50 Content-Disposition: inline\r
  51 In-Reply-To: <87sip3t37f.fsf@qmul.ac.uk>\r
  52 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  53 X-Brightmail-Tracker:\r
  54  H4sIAAAAAAAAA+NgFmpkleLIzCtJLcpLzFFi42IR4hRV1mUPiQw2eL3JymL1XB6L6zdnMjsw\r
  55         eeycdZfd49mqW8wBTFFcNimpOZllqUX6dglcGbMn72UpuKxVsWcbbwPjQsUuRg4OCQETiXX7\r
  56         8rsYOYFMMYkL99azdTFycQgJzGaS6Nx7mh3C2cgo8eVZLwuEc5pJYkHLFyYIZwmjxM69s1lB\r
  57         RrEIqEqsvWQNMopNQENi2/7ljCC2iICOxO1DC9hBbGYBaYlvv5uZQGxhgSiJrp3P2UBsXqCa\r
  58         bWfnQ22byijxdeYMJoiEoMTJmU9YIJq1JG78e8kEsgtk0PJ/HCBhTqBdn9p3s4LYogIqElNO\r
  59         bmObwCg0C0n3LCTdsxC6FzAyr2KUTcmt0s1NzMwpTk3WLU5OzMtLLdI11cvNLNFLTSndxAgK\r
  60         aXYXpR2MPw8qHWIU4GBU4uF9IR8ZLMSaWFZcmXuIUZKDSUmUN84PKMSXlJ9SmZFYnBFfVJqT\r
  61         WnyIUYKDWUmEd50tUI43JbGyKrUoHyYlzcGiJM771toqWEggPbEkNTs1tSC1CCYrw8GhJMFr\r
  62         FwzUKFiUmp5akZaZU4KQZuLgBBnOAzQ8JAhkeHFBYm5xZjpE/hSjopQ4bwZIswBIIqM0D64X\r
  63         lnJeMYoDvSLMWw5SxQNMV3Ddr4AGMwENLpgQDjK4JBEhJdXAqL//h+LHbdxJ0Y9WvDJ86eOT\r
  64         ZLj4gn8SY5Vq6QxhdVmvFqvggPv8ut/5uPeZfzb8I+XErci71D9/1pxzi5gC75dJhJ/JkpsY\r
  65         KhBQ9qH33faNwTsk+SKeCxsf3XRrhwDD0SvVR/9zxC3bHWBbqWzGe3fD9OJLaoZZthNnf8tv\r
  66         NjHh3776mpWpEktxRqKhFnNRcSIAo6iODBQDAAA=\r
  67 Cc: notmuch@notmuchmail.org\r
  68 X-BeenThere: notmuch@notmuchmail.org\r
  69 X-Mailman-Version: 2.1.13\r
  70 Precedence: list\r
  71 List-Id: "Use and development of the notmuch mail system."\r
  72         <notmuch.notmuchmail.org>\r
  73 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  74         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  75 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  76 List-Post: <mailto:notmuch@notmuchmail.org>\r
  77 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  78 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  79         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  80 X-List-Received-Date: Thu, 24 Apr 2014 18:12:30 -0000\r
  81 \r
  82 Quoth Mark Walters on Apr 24 at 11:46 am:\r
  83 > \r
  84 > On Mon, 21 Apr 2014, Austin Clements <amdragon@MIT.EDU> wrote:\r
  85 > > (The actual code change here is small, but requires re-indenting\r
  86 > > existing code.)\r
  87 > > ---\r
  88 > >  emacs/notmuch-lib.el | 52 ++++++++++++++++++++++++++++++----------------------\r
  89 > >  1 file changed, 30 insertions(+), 22 deletions(-)\r
  90 > >\r
  91 > > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el\r
  92 > > index fc67b14..fee8512 100644\r
  93 > > --- a/emacs/notmuch-lib.el\r
  94 > > +++ b/emacs/notmuch-lib.el\r
  95 > > @@ -503,33 +503,39 @@ (defun notmuch-parts-filter-by-type (parts type)\r
  96 > >     (lambda (part) (notmuch-match-content-type (plist-get part :content-type) type))\r
  97 > >     parts))\r
  98 > >  \r
  99 > > -(defun notmuch-get-bodypart-binary (msg part process-crypto)\r
 100 > > +(defun notmuch-get-bodypart-binary (msg part process-crypto &optional cache)\r
 101 > >    "Return the unprocessed content of PART in MSG as a unibyte string.\r
 102 > >  \r
 103 > >  This returns the \"raw\" content of the given part after content\r
 104 > >  transfer decoding, but with no further processing (see the\r
 105 > >  discussion of --format=raw in man notmuch-show).  In particular,\r
 106 > >  this does no charset conversion."\r
 107 > > -  (let ((args `("show" "--format=raw"\r
 108 > > -           ,(format "--part=%d" (plist-get part :id))\r
 109 > > -           ,@(when process-crypto '("--decrypt"))\r
 110 > > -           ,(notmuch-id-to-query (plist-get msg :id)))))\r
 111 > > -    (with-temp-buffer\r
 112 > > -      ;; Emacs internally uses a UTF-8-like multibyte string\r
 113 > > -      ;; representation by default (regardless of the coding system,\r
 114 > > -      ;; which only affects how it goes from outside data to this\r
 115 > > -      ;; internal representation).  This *almost* never matters.\r
 116 > > -      ;; Annoyingly, it does matter if we use this data in an image\r
 117 > > -      ;; descriptor, since Emacs will use its internal data buffer\r
 118 > > -      ;; directly and this multibyte representation corrupts binary\r
 119 > > -      ;; image formats.  Since the caller is asking for binary data, a\r
 120 > > -      ;; unibyte string is a more appropriate representation anyway.\r
 121 > > -      (set-buffer-multibyte nil)\r
 122 > > -      (let ((coding-system-for-read 'no-conversion))\r
 123 > > -   (apply #'call-process notmuch-command nil '(t nil) nil args)\r
 124 > > -   (buffer-string)))))\r
 125 > > -\r
 126 > > -(defun notmuch-get-bodypart-text (msg part process-crypto)\r
 127 > > +  (let ((data (plist-get part :binary-content)))\r
 128 > > +    (when (not data)\r
 129 > > +      (let ((args `("show" "--format=raw"\r
 130 > > +               ,(format "--part=%d" (plist-get part :id))\r
 131 > > +               ,@(when process-crypto '("--decrypt"))\r
 132 > > +               ,(notmuch-id-to-query (plist-get msg :id)))))\r
 133 > > +   (with-temp-buffer\r
 134 > > +     ;; Emacs internally uses a UTF-8-like multibyte string\r
 135 > > +     ;; representation by default (regardless of the coding\r
 136 > > +     ;; system, which only affects how it goes from outside data\r
 137 > > +     ;; to this internal representation).  This *almost* never\r
 138 > > +     ;; matters.  Annoyingly, it does matter if we use this data\r
 139 > > +     ;; in an image descriptor, since Emacs will use its internal\r
 140 > > +     ;; data buffer directly and this multibyte representation\r
 141 > > +     ;; corrupts binary image formats.  Since the caller is\r
 142 > > +     ;; asking for binary data, a unibyte string is a more\r
 143 > > +     ;; appropriate representation anyway.\r
 144 > > +     (set-buffer-multibyte nil)\r
 145 > > +     (let ((coding-system-for-read 'no-conversion))\r
 146 > > +       (apply #'call-process notmuch-command nil '(t nil) nil args)\r
 147 > > +       (setq data (buffer-string)))))\r
 148 > > +      (when cache\r
 149 > > +   (plist-put part :binary-content data)))\r
 150 > > +    data))\r
 151 > \r
 152 > I am a little puzzled by this but that could be lack of familiarity with\r
 153 > elisp. As far as I can see plist-put will sometimes modify the original\r
 154 > plist and sometimes return a new plist. If the latter happens then I\r
 155 > think it works out as if we hadn't cached anything as the part passed to\r
 156 > the function is unmodified. That might not matter in this case (though I\r
 157 > find the lack of determinism disturbing).\r
 158 > \r
 159 > Or is something else going on?\r
 160 \r
 161 No, your familiarity with Elisp serves you well.  I'm completely\r
 162 cheating here.  According to the specification of plist-put, it's\r
 163 allowed to return a new list but in reality this only happens when the\r
 164 original plist is nil.  We lean on this already all over the\r
 165 notmuch-emacs code, but maybe that doesn't excuse me adding one more\r
 166 cheat.\r
 167 \r
 168 I could add a comment here explaining what's going on, I could\r
 169 manually do the list insertion in a way that's guaranteed to mutate it\r
 170 in place, or I could add a nil :binary-content property when parts are\r
 171 created (since plist-put is guaranteed to mutate existing keys in\r
 172 place).\r
 173 \r
 174 > Best wishes\r
 175 > \r
 176 > Mark\r
 177 > \r
 178 > \r
 179 > \r
 180 > > +\r
 181 > > +(defun notmuch-get-bodypart-text (msg part process-crypto &optional cache)\r
 182 > >    "Return the text content of PART in MSG.\r
 183 > >  \r
 184 > >  This returns the content of the given part as a multibyte Lisp\r
 185 > > @@ -546,7 +552,9 @@ (defun notmuch-get-bodypart-text (msg part process-crypto)\r
 186 > >          (npart (apply #'notmuch-call-notmuch-sexp args)))\r
 187 > >     (setq content (plist-get npart :content))\r
 188 > >     (when (not content)\r
 189 > > -     (error "Internal error: No :content from %S" args))))\r
 190 > > +     (error "Internal error: No :content from %S" args)))\r
 191 > > +      (when cache\r
 192 > > +   (plist-put part :content content)))\r
 193 > >      content))\r
 194 > >  \r
 195 > >  ;; Workaround: The call to `mm-display-part' below triggers a bug in\r