[PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text}
On Fri, 25 Apr 2014, Mark Walters wrote:
> On Thu, 24 Apr 2014, Austin Clements wrote:
>> 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
>>> > 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).
>
> I think a comment is fine.
Done.
> (Incidentally what is the best way of
Re: [PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text}
On Fri, 25 Apr 2014, Mark Walters markwalters1...@gmail.com wrote: On Thu, 24 Apr 2014, Austin Clements amdra...@mit.edu wrote: Quoth Mark Walters on Apr 24 at 11:46 am: On Mon, 21 Apr 2014, Austin Clements amdra...@mit.edu 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). I think a comment is fine. Done. (Incidentally what is the best way of telling if emacs has changed an object or returned a new one for other commands? Something like (setq oldobject object) (setq object (operation-on object)) (if (eq object oldobject) ... )) If `eq' returns t, it definitely returned the original object, though it may or may not have modified
[PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text}
On Thu, 24 Apr 2014, Austin Clements wrote:
> 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
>> > 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).
I think a comment is fine.
(Incidentally what is the best way of telling if emacs has changed an
object or returned a new one for other commands? Something like (setq
oldobject object) (setq
Re: [PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text}
On Thu, 24 Apr 2014, Austin Clements amdra...@mit.edu wrote: Quoth Mark Walters on Apr 24 at 11:46 am: On Mon, 21 Apr 2014, Austin Clements amdra...@mit.edu 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). I think a comment is fine. (Incidentally what is the best way of telling if emacs has changed an object or returned a new one for other commands? Something like (setq oldobject object) (setq object (operation-on object)) (if (eq object oldobject) ... )) Also, I think the function should have a comment about the lifetime of the caching. I think in some cases the addition of :binary-content could occur on load
[PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text}
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
> > 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 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)
>
[PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text}
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 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?
Best wishes
Mark
> +
> +(defun notmuch-get-bodypart-text (msg part process-crypto 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
> --
> 1.9.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text}
On Mon, 21 Apr 2014, Austin Clements amdra...@mit.edu 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? 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 -- 1.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text}
Quoth Mark Walters on Apr 24 at 11:46 am: On Mon, 21 Apr 2014, Austin Clements amdra...@mit.edu 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
[PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text}
(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 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))
+
+(defun notmuch-get-bodypart-text (msg part process-crypto 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
--
1.9.1
[PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text}
(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)) + +(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 -- 1.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
