[PATCH 02/11] test: New tests for Emacs charset handling

2014-04-24 Thread Mark Walters

On Mon, 21 Apr 2014, Austin Clements  wrote:
> The test of viewing 8bit messages is known-broken.  The rest pass, but
> for very fragile reasons.  The next several commits will fix the
> known-broken test and make our charset handling robust.

Hi 

On one of my systems one of these (non-broken) tests fails. I am not
sure whether I messed up my emacs/environment when doing stuff remotely
recently so it could just be my system


> ---
>  test/T455-emacs-charsets.sh | 141 
> 
>  test/test-lib.el|   4 +-
>  2 files changed, 144 insertions(+), 1 deletion(-)
>  create mode 100755 test/T455-emacs-charsets.sh
>
> diff --git a/test/T455-emacs-charsets.sh b/test/T455-emacs-charsets.sh
> new file mode 100755
> index 000..a42a1d2
> --- /dev/null
> +++ b/test/T455-emacs-charsets.sh
> @@ -0,0 +1,141 @@
> +#!/usr/bin/env bash
> +
> +test_description="emacs notmuch-show charset handling"
> +. ./test-lib.sh
> +
> +
> +UTF8_YEN=$'\xef\xbf\xa5'
> +BIG5_YEN=$'\xa2\x44'
> +
> +# Add four messages with unusual encoding requirements:
> +#
> +# 1) text/plain in quoted-printable big5
> +generate_message \
> +[id]=test-plain at example.com \
> +'[content-type]="text/plain; charset=big5"' \
> +'[content-transfer-encoding]=quoted-printable' \
> +'[body]="Yen: =A2=44"'
> +
> +# 2) text/plain in 8bit big5
> +generate_message \
> +[id]=test-plain-8bit at example.com \
> +'[content-type]="text/plain; charset=big5"' \
> +'[content-transfer-encoding]=8bit' \
> +'[body]="Yen: '$BIG5_YEN'"'
> +
> +# 3) text/html in quoted-printable big5
> +generate_message \
> +[id]=test-html at example.com \
> +'[content-type]="text/html; charset=big5"' \
> +'[content-transfer-encoding]=quoted-printable' \
> +'[body]="Yen: =A2=44"'
> +
> +# 4) application/octet-stream in quoted-printable of big5 text
> +generate_message \
> +[id]=test-binary at example.com \
> +'[content-type]="application/octet-stream"' \
> +'[content-transfer-encoding]=quoted-printable' \
> +'[body]="Yen: =A2=44"'
> +
> +notmuch new > /dev/null
> +
> +# Test rendering
> +
> +test_begin_subtest "Text parts are decoded when rendering"
> +test_emacs '(notmuch-show "id:test-plain at example.com")
> + (test-visible-output "OUTPUT.raw")'
> +awk 'show {print} /^$/ {show=1}' < OUTPUT.raw > OUTPUT
> +cat  +Yen: $UTF8_YEN
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "8bit text parts are decoded when rendering"
> +test_emacs '(notmuch-show "id:test-plain-8bit at example.com")
> + (test-visible-output "OUTPUT.raw")'
> +awk 'show {print} /^$/ {show=1}' < OUTPUT.raw > OUTPUT
> +cat  +Yen: $UTF8_YEN
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "HTML parts are decoded when rendering"
> +test_emacs '(notmuch-show "id:test-html at example.com")
> + (test-visible-output "OUTPUT.raw")'
> +awk 'show {print} /^$/ {show=1}' < OUTPUT.raw > OUTPUT
> +cat  +[ text/html ]
> +Yen: $UTF8_YEN
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED

It's this test: I get an extra newline after the UFT8_YEN. 

This is with emacs 23.4.1 on a Debian wheezy(ish) system.

But as said it could be my fault: I think some odd things happened when
I tried to install emacs 24 and 23 simultaneously to help testing
things.

Best wishes

Mark


> +
> +# Test saving
> +
> +test_begin_subtest "Text parts are not decoded when saving"
> +rm -f part
> +test_emacs '(notmuch-show "id:test-plain at example.com")
> + (search-forward "Yen")
> + (let ((standard-input "\"part\""))
> +(notmuch-show-save-part))'
> +cat  +Yen: $BIG5_YEN
> +EOF
> +test_expect_equal_file part EXPECTED
> +
> +test_begin_subtest "8bit text parts are not decoded when saving"
> +rm -f part
> +test_emacs '(notmuch-show "id:test-plain-8bit at example.com")
> + (search-forward "Yen")
> + (let ((standard-input "\"part\""))
> +(notmuch-show-save-part))'
> +cat  +Yen: $BIG5_YEN
> +EOF
> +test_expect_equal_file part EXPECTED
> +
> +test_begin_subtest "HTML parts are not decoded when saving"
> +rm -f part
> +test_emacs '(notmuch-show "id:test-html at example.com")
> + (search-forward "Yen")
> + (let ((standard-input "\"part\""))
> +(notmuch-show-save-part))'
> +cat  +Yen: $BIG5_YEN
> +EOF
> +test_expect_equal_file part EXPECTED
> +
> +test_begin_subtest "Binary parts are not decoded when saving"
> +rm -f part
> +test_emacs '(notmuch-show "id:test-binary at example.com")
> + (search-forward "application/")
> + (let ((standard-input "\"part\""))
> +(notmuch-show-save-part))'
> +cat  +Yen: $BIG5_YEN
> +EOF
> +test_expect_equal_file part EXPECTED
> +
> +# Test message viewing
> +
> +test_begin_subtest "Text message are not decoded when viewing"
> +test_emacs '(notmuch-show "id:test-plain at 

[PATCH 1/1] building from git: use --abbrev=7 for version string

2014-04-24 Thread David Bremner
Tomi Ollila  writes:

> Users may have set core.abbrev=n, where n != 7 in their git config
> file(s) which would give them different than expected version strings
> when building notmuch from git. This fixes the commit hash part of
> version string to 7 hexadecimal values.

pushed to master

d


[PATCH 02/11] test: New tests for Emacs charset handling

2014-04-24 Thread Austin Clements
Quoth Mark Walters on Apr 24 at  3:38 pm:
> 
> On Mon, 21 Apr 2014, Austin Clements  wrote:
> > The test of viewing 8bit messages is known-broken.  The rest pass, but
> > for very fragile reasons.  The next several commits will fix the
> > known-broken test and make our charset handling robust.
> 
> Hi 
> 
> On one of my systems one of these (non-broken) tests fails. I am not
> sure whether I messed up my emacs/environment when doing stuff remotely
> recently so it could just be my system
> 
> 
> > ---
> >  test/T455-emacs-charsets.sh | 141 
> > 
> >  test/test-lib.el|   4 +-
> >  2 files changed, 144 insertions(+), 1 deletion(-)
> >  create mode 100755 test/T455-emacs-charsets.sh
> >
> > diff --git a/test/T455-emacs-charsets.sh b/test/T455-emacs-charsets.sh
> > new file mode 100755
> > index 000..a42a1d2
> > --- /dev/null
> > +++ b/test/T455-emacs-charsets.sh
> > @@ -0,0 +1,141 @@
> > +#!/usr/bin/env bash
> > +
> > +test_description="emacs notmuch-show charset handling"
> > +. ./test-lib.sh
> > +
> > +
> > +UTF8_YEN=$'\xef\xbf\xa5'
> > +BIG5_YEN=$'\xa2\x44'
> > +
> > +# Add four messages with unusual encoding requirements:
> > +#
> > +# 1) text/plain in quoted-printable big5
> > +generate_message \
> > +[id]=test-plain at example.com \
> > +'[content-type]="text/plain; charset=big5"' \
> > +'[content-transfer-encoding]=quoted-printable' \
> > +'[body]="Yen: =A2=44"'
> > +
> > +# 2) text/plain in 8bit big5
> > +generate_message \
> > +[id]=test-plain-8bit at example.com \
> > +'[content-type]="text/plain; charset=big5"' \
> > +'[content-transfer-encoding]=8bit' \
> > +'[body]="Yen: '$BIG5_YEN'"'
> > +
> > +# 3) text/html in quoted-printable big5
> > +generate_message \
> > +[id]=test-html at example.com \
> > +'[content-type]="text/html; charset=big5"' \
> > +'[content-transfer-encoding]=quoted-printable' \
> > +'[body]="Yen: =A2=44"'
> > +
> > +# 4) application/octet-stream in quoted-printable of big5 text
> > +generate_message \
> > +[id]=test-binary at example.com \
> > +'[content-type]="application/octet-stream"' \
> > +'[content-transfer-encoding]=quoted-printable' \
> > +'[body]="Yen: =A2=44"'
> > +
> > +notmuch new > /dev/null
> > +
> > +# Test rendering
> > +
> > +test_begin_subtest "Text parts are decoded when rendering"
> > +test_emacs '(notmuch-show "id:test-plain at example.com")
> > +   (test-visible-output "OUTPUT.raw")'
> > +awk 'show {print} /^$/ {show=1}' < OUTPUT.raw > OUTPUT
> > +cat  > +Yen: $UTF8_YEN
> > +EOF
> > +test_expect_equal_file OUTPUT EXPECTED
> > +
> > +test_begin_subtest "8bit text parts are decoded when rendering"
> > +test_emacs '(notmuch-show "id:test-plain-8bit at example.com")
> > +   (test-visible-output "OUTPUT.raw")'
> > +awk 'show {print} /^$/ {show=1}' < OUTPUT.raw > OUTPUT
> > +cat  > +Yen: $UTF8_YEN
> > +EOF
> > +test_expect_equal_file OUTPUT EXPECTED
> > +
> > +test_begin_subtest "HTML parts are decoded when rendering"
> > +test_emacs '(notmuch-show "id:test-html at example.com")
> > +   (test-visible-output "OUTPUT.raw")'
> > +awk 'show {print} /^$/ {show=1}' < OUTPUT.raw > OUTPUT
> > +cat  > +[ text/html ]
> > +Yen: $UTF8_YEN
> > +EOF
> > +test_expect_equal_file OUTPUT EXPECTED
> 
> It's this test: I get an extra newline after the UFT8_YEN. 
> 
> This is with emacs 23.4.1 on a Debian wheezy(ish) system.
> 
> But as said it could be my fault: I think some odd things happened when
> I tried to install emacs 24 and 23 simultaneously to help testing
> things.

I can reproduce this.  Tests that involve HTML rendering are always
finicky like this since there are so many different HTML renderers.
Maybe I could normalize the spacing?  sed '/^$/d;s/  */ /g' or so?

> Best wishes
> 
> Mark
> 
> 
> > +
> > +# Test saving
> > +
> > +test_begin_subtest "Text parts are not decoded when saving"
> > +rm -f part
> > +test_emacs '(notmuch-show "id:test-plain at example.com")
> > +   (search-forward "Yen")
> > +   (let ((standard-input "\"part\""))
> > +  (notmuch-show-save-part))'
> > +cat  > +Yen: $BIG5_YEN
> > +EOF
> > +test_expect_equal_file part EXPECTED
> > +
> > +test_begin_subtest "8bit text parts are not decoded when saving"
> > +rm -f part
> > +test_emacs '(notmuch-show "id:test-plain-8bit at example.com")
> > +   (search-forward "Yen")
> > +   (let ((standard-input "\"part\""))
> > +  (notmuch-show-save-part))'
> > +cat  > +Yen: $BIG5_YEN
> > +EOF
> > +test_expect_equal_file part EXPECTED
> > +
> > +test_begin_subtest "HTML parts are not decoded when saving"
> > +rm -f part
> > +test_emacs '(notmuch-show "id:test-html at example.com")
> > +   (search-forward "Yen")
> > +   (let ((standard-input "\"part\""))
> > +  (notmuch-show-save-part))'
> > +cat  > +Yen: $BIG5_YEN
> > +EOF
> > +test_expect_equal_file part EXPECTED
> > 

[PATCH 08/11] emacs: Support caching in notmuch-get-bodypart-{binary, text}

2014-04-24 Thread Austin Clements
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}

2014-04-24 Thread Mark Walters

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}

2014-04-24 Thread Mark Walters

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] Add configurable changed tag to messages that have been changed on disk

2014-04-24 Thread David Mazieres expires 2014-07-22 PDT
Austin Clements amdra...@mit.edu writes:

 I'd like to have efficient change detection, too.  In my case, I'd
 like to use it to support efficient live search and show updates.  The
 design I'd sketched out for that used a log rather than ctimes, and
 I'm curious if you have thoughts on the relative merits and
 suitability for tag sync of these approaches.

Both logging ctime are very general mechanisms than can solve many
problems.  For example, is there still an issue that pressing * in
emacs notmuch-search mode can affect messages that are not visible if
someone ran notmuch new in a different window?  ctimes would be one way
to solve this.  (Conservatively exempt any messages that have changed
since the displayed search was run.)

 I'd been leaning toward logging because it can capture changes to
 things that aren't represented as documents in the database, such as
 thread membership.  This probably doesn't matter for synchronization,
 but it makes it much easier to figure out which threads in thread
 search results need to be refreshed.  A log can also capture message
 deletion easily, while ctimes would require tombstones (which may be a
 good idea for other reasons [1]).

 On the other hand, logging is obviously more mechanism than ctimes,
 and probably requires some garbage collection story.

The advantage of ctime over logging is that the interface is super
simple and intuitive.  How would the interface to the log work?

In terms of implementation, have you thought about leveraging the
XAPIAN_MAX_CHANGESETS mechanism to produce your logs?

David
___
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}

2014-04-24 Thread Austin Clements
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