From: Austin Clements Date: Thu, 24 Apr 2014 18:12:17 +0000 (+2000) Subject: Re: [PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text} X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=0f1170884567769226bba63c6e0d3134ec42b938;p=notmuch-archives.git Re: [PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text} --- diff --git a/c1/e60daf79134afdbce66971976cd3731c953634 b/c1/e60daf79134afdbce66971976cd3731c953634 new file mode 100644 index 000000000..4a0589306 --- /dev/null +++ b/c1/e60daf79134afdbce66971976cd3731c953634 @@ -0,0 +1,195 @@ +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 5A437431FAF + for ; Thu, 24 Apr 2014 11:12:30 -0700 (PDT) +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 JmVCETnxE4pR for ; + Thu, 24 Apr 2014 11:12:24 -0700 (PDT) +Received: from dmz-mailsec-scanner-5.mit.edu (dmz-mailsec-scanner-5.mit.edu + [18.7.68.34]) + by olra.theworths.org (Postfix) with ESMTP id 94479431FAE + for ; Thu, 24 Apr 2014 11:12:24 -0700 (PDT) +X-AuditID: 12074422-f79186d00000135a-cd-53595407e39e +Received: from mailhub-auth-1.mit.edu ( [18.9.21.35]) + (using TLS with cipher AES256-SHA (256/256 bits)) + (Client did not present a certificate) + by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP + id 4B.79.04954.70459535; Thu, 24 Apr 2014 14:12:23 -0400 (EDT) +Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) + by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id s3OICM0Z023927; + Thu, 24 Apr 2014 14:12:23 -0400 +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.8/8.12.4) with ESMTP id s3OICKge007507 + (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); + Thu, 24 Apr 2014 14:12:21 -0400 +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80) + (envelope-from ) + id 1WdO8B-0005Qt-Kd; Thu, 24 Apr 2014 14:12:19 -0400 +Date: Thu, 24 Apr 2014 14:12:17 -0400 +From: Austin Clements +To: Mark Walters +Subject: Re: [PATCH 08/11] emacs: Support caching in + notmuch-get-bodypart-{binary, text} +Message-ID: <20140424181216.GO25817@mit.edu> +References: <1398105468-14317-1-git-send-email-amdragon@mit.edu> + <1398105468-14317-9-git-send-email-amdragon@mit.edu> + <87sip3t37f.fsf@qmul.ac.uk> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline +In-Reply-To: <87sip3t37f.fsf@qmul.ac.uk> +User-Agent: Mutt/1.5.21 (2010-09-15) +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFmpkleLIzCtJLcpLzFFi42IR4hRV1mUPiQw2eL3JymL1XB6L6zdnMjsw + eeycdZfd49mqW8wBTFFcNimpOZllqUX6dglcGbMn72UpuKxVsWcbbwPjQsUuRg4OCQETiXX7 + 8rsYOYFMMYkL99azdTFycQgJzGaS6Nx7mh3C2cgo8eVZLwuEc5pJYkHLFyYIZwmjxM69s1lB + RrEIqEqsvWQNMopNQENi2/7ljCC2iICOxO1DC9hBbGYBaYlvv5uZQGxhgSiJrp3P2UBsXqCa + bWfnQ22byijxdeYMJoiEoMTJmU9YIJq1JG78e8kEsgtk0PJ/HCBhTqBdn9p3s4LYogIqElNO + bmObwCg0C0n3LCTdsxC6FzAyr2KUTcmt0s1NzMwpTk3WLU5OzMtLLdI11cvNLNFLTSndxAgK + aXYXpR2MPw8qHWIU4GBU4uF9IR8ZLMSaWFZcmXuIUZKDSUmUN84PKMSXlJ9SmZFYnBFfVJqT + WnyIUYKDWUmEd50tUI43JbGyKrUoHyYlzcGiJM771toqWEggPbEkNTs1tSC1CCYrw8GhJMFr + FwzUKFiUmp5akZaZU4KQZuLgBBnOAzQ8JAhkeHFBYm5xZjpE/hSjopQ4bwZIswBIIqM0D64X + lnJeMYoDvSLMWw5SxQNMV3Ddr4AGMwENLpgQDjK4JBEhJdXAqL//h+LHbdxJ0Y9WvDJ86eOT + ZLj4gn8SY5Vq6QxhdVmvFqvggPv8ut/5uPeZfzb8I+XErci71D9/1pxzi5gC75dJhJ/JkpsY + KhBQ9qH33faNwTsk+SKeCxsf3XRrhwDD0SvVR/9zxC3bHWBbqWzGe3fD9OJLaoZZthNnf8tv + NjHh3776mpWpEktxRqKhFnNRcSIAo6iODBQDAAA= +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: Thu, 24 Apr 2014 18:12:30 -0000 + +Quoth Mark Walters on Apr 24 at 11:46 am: +> +> On Mon, 21 Apr 2014, Austin Clements wrote: +> > (The actual code change here is small, but requires re-indenting +> > existing code.) +> > --- +> > emacs/notmuch-lib.el | 52 ++++++++++++++++++++++++++++++---------------------- +> > 1 file changed, 30 insertions(+), 22 deletions(-) +> > +> > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el +> > index fc67b14..fee8512 100644 +> > --- a/emacs/notmuch-lib.el +> > +++ b/emacs/notmuch-lib.el +> > @@ -503,33 +503,39 @@ (defun notmuch-parts-filter-by-type (parts type) +> > (lambda (part) (notmuch-match-content-type (plist-get part :content-type) type)) +> > parts)) +> > +> > -(defun notmuch-get-bodypart-binary (msg part process-crypto) +> > +(defun notmuch-get-bodypart-binary (msg part process-crypto &optional cache) +> > "Return the unprocessed content of PART in MSG as a unibyte string. +> > +> > This returns the \"raw\" content of the given part after content +> > transfer decoding, but with no further processing (see the +> > discussion of --format=raw in man notmuch-show). In particular, +> > this does no charset conversion." +> > - (let ((args `("show" "--format=raw" +> > - ,(format "--part=%d" (plist-get part :id)) +> > - ,@(when process-crypto '("--decrypt")) +> > - ,(notmuch-id-to-query (plist-get msg :id))))) +> > - (with-temp-buffer +> > - ;; Emacs internally uses a UTF-8-like multibyte string +> > - ;; representation by default (regardless of the coding system, +> > - ;; which only affects how it goes from outside data to this +> > - ;; internal representation). This *almost* never matters. +> > - ;; Annoyingly, it does matter if we use this data in an image +> > - ;; descriptor, since Emacs will use its internal data buffer +> > - ;; directly and this multibyte representation corrupts binary +> > - ;; image formats. Since the caller is asking for binary data, a +> > - ;; unibyte string is a more appropriate representation anyway. +> > - (set-buffer-multibyte nil) +> > - (let ((coding-system-for-read 'no-conversion)) +> > - (apply #'call-process notmuch-command nil '(t nil) nil args) +> > - (buffer-string))))) +> > - +> > -(defun notmuch-get-bodypart-text (msg part process-crypto) +> > + (let ((data (plist-get part :binary-content))) +> > + (when (not data) +> > + (let ((args `("show" "--format=raw" +> > + ,(format "--part=%d" (plist-get part :id)) +> > + ,@(when process-crypto '("--decrypt")) +> > + ,(notmuch-id-to-query (plist-get msg :id))))) +> > + (with-temp-buffer +> > + ;; Emacs internally uses a UTF-8-like multibyte string +> > + ;; representation by default (regardless of the coding +> > + ;; system, which only affects how it goes from outside data +> > + ;; to this internal representation). This *almost* never +> > + ;; matters. Annoyingly, it does matter if we use this data +> > + ;; in an image descriptor, since Emacs will use its internal +> > + ;; data buffer directly and this multibyte representation +> > + ;; corrupts binary image formats. Since the caller is +> > + ;; asking for binary data, a unibyte string is a more +> > + ;; appropriate representation anyway. +> > + (set-buffer-multibyte nil) +> > + (let ((coding-system-for-read 'no-conversion)) +> > + (apply #'call-process notmuch-command nil '(t nil) nil args) +> > + (setq data (buffer-string))))) +> > + (when cache +> > + (plist-put part :binary-content data))) +> > + data)) +> +> I am a little puzzled by this but that could be lack of familiarity with +> elisp. As far as I can see plist-put will sometimes modify the original +> plist and sometimes return a new plist. If the latter happens then I +> think it works out as if we hadn't cached anything as the part passed to +> the function is unmodified. That might not matter in this case (though I +> find the lack of determinism disturbing). +> +> Or is something else going on? + +No, your familiarity with Elisp serves you well. I'm completely +cheating here. According to the specification of plist-put, it's +allowed to return a new list but in reality this only happens when the +original plist is nil. We lean on this already all over the +notmuch-emacs code, but maybe that doesn't excuse me adding one more +cheat. + +I could add a comment here explaining what's going on, I could +manually do the list insertion in a way that's guaranteed to mutate it +in place, or I could add a nil :binary-content property when parts are +created (since plist-put is guaranteed to mutate existing keys in +place). + +> Best wishes +> +> Mark +> +> +> +> > + +> > +(defun notmuch-get-bodypart-text (msg part process-crypto &optional cache) +> > "Return the text content of PART in MSG. +> > +> > This returns the content of the given part as a multibyte Lisp +> > @@ -546,7 +552,9 @@ (defun notmuch-get-bodypart-text (msg part process-crypto) +> > (npart (apply #'notmuch-call-notmuch-sexp args))) +> > (setq content (plist-get npart :content)) +> > (when (not content) +> > - (error "Internal error: No :content from %S" args)))) +> > + (error "Internal error: No :content from %S" args))) +> > + (when cache +> > + (plist-put part :content content))) +> > content)) +> > +> > ;; Workaround: The call to `mm-display-part' below triggers a bug in