unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] emacs: Make the part content available to the mm-inline* checks.
@ 2012-01-18 17:33 David Edmondson
  2012-01-18 17:35 ` David Edmondson
  2012-01-18 17:39 ` [PATCH v2] " David Edmondson
  0 siblings, 2 replies; 19+ messages in thread
From: David Edmondson @ 2012-01-18 17:33 UTC (permalink / raw)
  To: notmuch

The `mm-inlinable-p' and `mm-inlined-p' functions work better if they
have access to the data of the relevant part, so load that content
before calling either function.

This fixes the display of attached image/jpeg parts, for example.
---

I dropped this on the floor after discussing it in #notmuch, sorry!

 emacs/notmuch-show.el |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 2df8d3b..71309c3 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -330,14 +330,17 @@ current buffer, if possible."
     (with-temp-buffer
       (let* ((charset (plist-get part :content-charset))
 	     (handle (mm-make-handle (current-buffer) `(,content-type (charset . ,charset)))))
-	(if (and (mm-inlinable-p handle)
-		 (mm-inlined-p handle))
-	    (let ((content (notmuch-show-get-bodypart-content msg part nth)))
-	      (insert content)
-	      (set-buffer display-buffer)
-	      (mm-display-part handle)
-	      t)
-	  nil)))))
+	(insert (notmuch-show-get-bodypart-content msg part nth))
+	(when (and (mm-inlinable-p handle)
+		   (mm-inlined-p handle))
+	  (set-buffer display-buffer)
+
+	  ;; Nonsense required to have the new gnus `shr' HTML
+	  ;; display code work.
+	  (let ((gnus-inhibit-images nil))
+	    (makunbound 'gnus-summary-buffer) ; Blech.
+	    (mm-display-part handle))
+	  t)))))
 
 (defvar notmuch-show-multipart/alternative-discouraged
   '(
-- 
1.7.8.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] emacs: Make the part content available to the mm-inline* checks.
  2012-01-18 17:33 [PATCH] emacs: Make the part content available to the mm-inline* checks David Edmondson
@ 2012-01-18 17:35 ` David Edmondson
  2012-01-18 17:39 ` [PATCH v2] " David Edmondson
  1 sibling, 0 replies; 19+ messages in thread
From: David Edmondson @ 2012-01-18 17:35 UTC (permalink / raw)
  To: notmuch

[-- Attachment #1: Type: text/plain, Size: 363 bytes --]

On Wed, 18 Jan 2012 17:33:13 +0000, David Edmondson <dme@dme.org> wrote:
> +	  ;; Nonsense required to have the new gnus `shr' HTML
> +	  ;; display code work.
> +	  (let ((gnus-inhibit-images nil))
> +	    (makunbound 'gnus-summary-buffer) ; Blech.
> +	    (mm-display-part handle))
> +	  t)))))

Bleugh. Sorry, don't apply this patch. I'll produce a clean one.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2] emacs: Make the part content available to the mm-inline* checks.
  2012-01-18 17:33 [PATCH] emacs: Make the part content available to the mm-inline* checks David Edmondson
  2012-01-18 17:35 ` David Edmondson
@ 2012-01-18 17:39 ` David Edmondson
  2012-01-18 18:04   ` Dmitry Kurochkin
  1 sibling, 1 reply; 19+ messages in thread
From: David Edmondson @ 2012-01-18 17:39 UTC (permalink / raw)
  To: notmuch

The `mm-inlinable-p' and `mm-inlined-p' functions work better if they
have access to the data of the relevant part, so load that content
before calling either function.

This fixes the display of attached image/jpeg parts, for example.
---

Removed the cruft that crept into the previous patch.

 emacs/notmuch-show.el |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 2df8d3b..f280df2 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -330,14 +330,12 @@ current buffer, if possible."
     (with-temp-buffer
       (let* ((charset (plist-get part :content-charset))
 	     (handle (mm-make-handle (current-buffer) `(,content-type (charset . ,charset)))))
-	(if (and (mm-inlinable-p handle)
-		 (mm-inlined-p handle))
-	    (let ((content (notmuch-show-get-bodypart-content msg part nth)))
-	      (insert content)
-	      (set-buffer display-buffer)
-	      (mm-display-part handle)
-	      t)
-	  nil)))))
+	(insert (notmuch-show-get-bodypart-content msg part nth))
+	(when (and (mm-inlinable-p handle)
+		   (mm-inlined-p handle))
+	  (set-buffer display-buffer)
+	  (mm-display-part handle)
+	  t)))))
 
 (defvar notmuch-show-multipart/alternative-discouraged
   '(
-- 
1.7.8.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] emacs: Make the part content available to the mm-inline* checks.
  2012-01-18 17:39 ` [PATCH v2] " David Edmondson
@ 2012-01-18 18:04   ` Dmitry Kurochkin
  2012-01-18 18:30     ` David Edmondson
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Kurochkin @ 2012-01-18 18:04 UTC (permalink / raw)
  To: David Edmondson, notmuch

Hi David.

On Wed, 18 Jan 2012 17:39:31 +0000, David Edmondson <dme@dme.org> wrote:
> The `mm-inlinable-p' and `mm-inlined-p' functions work better if they
> have access to the data of the relevant part, so load that content
> before calling either function.
> 
> This fixes the display of attached image/jpeg parts, for example.

Not so long ago I made an opposite change to avoid fetching useless
parts (e.g. audio files).  Looks like we need a better check here.  Can
we know from Content-Type if fetching a part body would be useful?

Regards,
  Dmitry

> ---
> 
> Removed the cruft that crept into the previous patch.
> 
>  emacs/notmuch-show.el |   14 ++++++--------
>  1 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 2df8d3b..f280df2 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -330,14 +330,12 @@ current buffer, if possible."
>      (with-temp-buffer
>        (let* ((charset (plist-get part :content-charset))
>  	     (handle (mm-make-handle (current-buffer) `(,content-type (charset . ,charset)))))
> -	(if (and (mm-inlinable-p handle)
> -		 (mm-inlined-p handle))
> -	    (let ((content (notmuch-show-get-bodypart-content msg part nth)))
> -	      (insert content)
> -	      (set-buffer display-buffer)
> -	      (mm-display-part handle)
> -	      t)
> -	  nil)))))
> +	(insert (notmuch-show-get-bodypart-content msg part nth))
> +	(when (and (mm-inlinable-p handle)
> +		   (mm-inlined-p handle))
> +	  (set-buffer display-buffer)
> +	  (mm-display-part handle)
> +	  t)))))
>  
>  (defvar notmuch-show-multipart/alternative-discouraged
>    '(
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] emacs: Make the part content available to the mm-inline* checks.
  2012-01-18 18:04   ` Dmitry Kurochkin
@ 2012-01-18 18:30     ` David Edmondson
  2012-01-18 19:00       ` Dmitry Kurochkin
  0 siblings, 1 reply; 19+ messages in thread
From: David Edmondson @ 2012-01-18 18:30 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

On Wed, 18 Jan 2012 22:04:36 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Wed, 18 Jan 2012 17:39:31 +0000, David Edmondson <dme@dme.org> wrote:
> > The `mm-inlinable-p' and `mm-inlined-p' functions work better if they
> > have access to the data of the relevant part, so load that content
> > before calling either function.
> > 
> > This fixes the display of attached image/jpeg parts, for example.
> 
> Not so long ago I made an opposite change to avoid fetching useless
> parts (e.g. audio files).  Looks like we need a better check here.  Can
> we know from Content-Type if fetching a part body would be useful?

What if `notmuch-show-insert-part-*/*' consulted a list of content-type
regexps to attempt to inline?

That would allow a sane default (("image/*" "text/*") perhaps), but also
allow more to be added to that list (or some to be removed), either by
code that detected the (in)ability to render it or the user.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] emacs: Make the part content available to the mm-inline* checks.
  2012-01-18 18:30     ` David Edmondson
@ 2012-01-18 19:00       ` Dmitry Kurochkin
  2012-01-18 19:35         ` Austin Clements
  2012-01-19  7:39         ` David Edmondson
  0 siblings, 2 replies; 19+ messages in thread
From: Dmitry Kurochkin @ 2012-01-18 19:00 UTC (permalink / raw)
  To: David Edmondson, notmuch

On Wed, 18 Jan 2012 18:30:36 +0000, David Edmondson <dme@dme.org> wrote:
> On Wed, 18 Jan 2012 22:04:36 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > On Wed, 18 Jan 2012 17:39:31 +0000, David Edmondson <dme@dme.org> wrote:
> > > The `mm-inlinable-p' and `mm-inlined-p' functions work better if they
> > > have access to the data of the relevant part, so load that content
> > > before calling either function.
> > > 
> > > This fixes the display of attached image/jpeg parts, for example.
> > 
> > Not so long ago I made an opposite change to avoid fetching useless
> > parts (e.g. audio files).  Looks like we need a better check here.  Can
> > we know from Content-Type if fetching a part body would be useful?
> 
> What if `notmuch-show-insert-part-*/*' consulted a list of content-type
> regexps to attempt to inline?
> 
> That would allow a sane default (("image/*" "text/*") perhaps), but also
> allow more to be added to that list (or some to be removed), either by
> code that detected the (in)ability to render it or the user.

Perhaps there is such a list in mm already?

Regards,
  Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] emacs: Make the part content available to the mm-inline* checks.
  2012-01-18 19:00       ` Dmitry Kurochkin
@ 2012-01-18 19:35         ` Austin Clements
  2012-01-18 19:59           ` Dmitry Kurochkin
  2012-01-20 20:13           ` Jameson Graef Rollins
  2012-01-19  7:39         ` David Edmondson
  1 sibling, 2 replies; 19+ messages in thread
From: Austin Clements @ 2012-01-18 19:35 UTC (permalink / raw)
  To: Dmitry Kurochkin; +Cc: notmuch

Quoth Dmitry Kurochkin on Jan 18 at 11:00 pm:
> On Wed, 18 Jan 2012 18:30:36 +0000, David Edmondson <dme@dme.org> wrote:
> > On Wed, 18 Jan 2012 22:04:36 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > On Wed, 18 Jan 2012 17:39:31 +0000, David Edmondson <dme@dme.org> wrote:
> > > > The `mm-inlinable-p' and `mm-inlined-p' functions work better if they
> > > > have access to the data of the relevant part, so load that content
> > > > before calling either function.
> > > > 
> > > > This fixes the display of attached image/jpeg parts, for example.
> > > 
> > > Not so long ago I made an opposite change to avoid fetching useless
> > > parts (e.g. audio files).  Looks like we need a better check here.  Can
> > > we know from Content-Type if fetching a part body would be useful?
> > 
> > What if `notmuch-show-insert-part-*/*' consulted a list of content-type
> > regexps to attempt to inline?
> > 
> > That would allow a sane default (("image/*" "text/*") perhaps), but also
> > allow more to be added to that list (or some to be removed), either by
> > code that detected the (in)ability to render it or the user.
> 
> Perhaps there is such a list in mm already?

Shouldn't we only be doing this for parts with inline (or not
attachment) content-disposition?  That's cheap to check.  Or do we
actually want things like image attachments to get inlined, despite
their disposition?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] emacs: Make the part content available to the mm-inline* checks.
  2012-01-18 19:35         ` Austin Clements
@ 2012-01-18 19:59           ` Dmitry Kurochkin
  2012-01-20 20:13           ` Jameson Graef Rollins
  1 sibling, 0 replies; 19+ messages in thread
From: Dmitry Kurochkin @ 2012-01-18 19:59 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

On Wed, 18 Jan 2012 14:35:01 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Dmitry Kurochkin on Jan 18 at 11:00 pm:
> > On Wed, 18 Jan 2012 18:30:36 +0000, David Edmondson <dme@dme.org> wrote:
> > > On Wed, 18 Jan 2012 22:04:36 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > > On Wed, 18 Jan 2012 17:39:31 +0000, David Edmondson <dme@dme.org> wrote:
> > > > > The `mm-inlinable-p' and `mm-inlined-p' functions work better if they
> > > > > have access to the data of the relevant part, so load that content
> > > > > before calling either function.
> > > > > 
> > > > > This fixes the display of attached image/jpeg parts, for example.
> > > > 
> > > > Not so long ago I made an opposite change to avoid fetching useless
> > > > parts (e.g. audio files).  Looks like we need a better check here.  Can
> > > > we know from Content-Type if fetching a part body would be useful?
> > > 
> > > What if `notmuch-show-insert-part-*/*' consulted a list of content-type
> > > regexps to attempt to inline?
> > > 
> > > That would allow a sane default (("image/*" "text/*") perhaps), but also
> > > allow more to be added to that list (or some to be removed), either by
> > > code that detected the (in)ability to render it or the user.
> > 
> > Perhaps there is such a list in mm already?
> 
> Shouldn't we only be doing this for parts with inline (or not
> attachment) content-disposition?  That's cheap to check.  Or do we
> actually want things like image attachments to get inlined, despite
> their disposition?

It may be good to have this behavior configurable.  I would like Emacs
to display all part types that it can, independent from
content-disposition.

Anyway, this is a separate issue.  In any case we want to fetch part
body only if it is useful.

Regards,
  Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] emacs: Make the part content available to the mm-inline* checks.
  2012-01-18 19:00       ` Dmitry Kurochkin
  2012-01-18 19:35         ` Austin Clements
@ 2012-01-19  7:39         ` David Edmondson
  2012-01-19  9:23           ` Dmitry Kurochkin
  1 sibling, 1 reply; 19+ messages in thread
From: David Edmondson @ 2012-01-19  7:39 UTC (permalink / raw)
  To: Dmitry Kurochkin; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 1798 bytes --]

On Wed, 18 Jan 2012 23:00:15 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > That would allow a sane default (("image/*" "text/*") perhaps), but also
> > allow more to be added to that list (or some to be removed), either by
> > code that detected the (in)ability to render it or the user.
> 
> Perhaps there is such a list in mm already?

There's a function which does almost exactly this - `mm-inlinable-p'. It
has a list of types and tests, `mm-inline-media-tests'. Some of those
tests require access to the part content to decide if the part is
inlinable. Many of them don't. The image/jpeg test _does_ want access to
the part content.

We're already using this function, of course.

`mm-inlined-p' is the corresponding "does the user want this part
inlined" test. It's much simpler and never looks at the part content.

Currently we merge those tests into one:

	(if (and (mm-inlinable-p handle)
		 (mm-inlined-p handle))

and have acquired part content either before or after the combined test.

Perhaps we could test `mm-inlined-p' first, then insert the content,
then test `mm-inlinable-p'? That way we would not acquire the content
for parts for which the user (or code) has selected not to inline the
content.

Currently `mm-inlined-p' suggests that the following should be inlined
by default:

    "image/.*" "text/.*" "message/delivery-status" "message/rfc822"
    "message/partial" "message/external-body" "application/emacs-lisp"
    "application/x-emacs-lisp" "application/pgp-signature"
    "application/x-pkcs7-signature" "application/pkcs7-signature"
    "application/x-pkcs7-mime" "application/pkcs7-mime"

These are the only types for which we'd acquire the part content for
examination by `mm-inlinable-p' (by default).

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] emacs: Make the part content available to the mm-inline* checks.
  2012-01-19  7:39         ` David Edmondson
@ 2012-01-19  9:23           ` Dmitry Kurochkin
  2012-01-19  9:34             ` [PATCH] emacs: Make the part content available to `mm-inlinable-p' David Edmondson
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Kurochkin @ 2012-01-19  9:23 UTC (permalink / raw)
  To: David Edmondson, notmuch

On Thu, 19 Jan 2012 07:39:32 +0000, David Edmondson <dme@dme.org> wrote:
> On Wed, 18 Jan 2012 23:00:15 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > That would allow a sane default (("image/*" "text/*") perhaps), but also
> > > allow more to be added to that list (or some to be removed), either by
> > > code that detected the (in)ability to render it or the user.
> > 
> > Perhaps there is such a list in mm already?
> 
> There's a function which does almost exactly this - `mm-inlinable-p'. It
> has a list of types and tests, `mm-inline-media-tests'. Some of those
> tests require access to the part content to decide if the part is
> inlinable. Many of them don't. The image/jpeg test _does_ want access to
> the part content.
> 
> We're already using this function, of course.
> 
> `mm-inlined-p' is the corresponding "does the user want this part
> inlined" test. It's much simpler and never looks at the part content.
> 
> Currently we merge those tests into one:
> 
> 	(if (and (mm-inlinable-p handle)
> 		 (mm-inlined-p handle))
> 
> and have acquired part content either before or after the combined test.
> 
> Perhaps we could test `mm-inlined-p' first, then insert the content,
> then test `mm-inlinable-p'? That way we would not acquire the content
> for parts for which the user (or code) has selected not to inline the
> content.
> 

Makes sense to me.

> Currently `mm-inlined-p' suggests that the following should be inlined
> by default:
> 
>     "image/.*" "text/.*" "message/delivery-status" "message/rfc822"
>     "message/partial" "message/external-body" "application/emacs-lisp"
>     "application/x-emacs-lisp" "application/pgp-signature"
>     "application/x-pkcs7-signature" "application/pkcs7-signature"
>     "application/x-pkcs7-mime" "application/pkcs7-mime"
> 
> These are the only types for which we'd acquire the part content for
> examination by `mm-inlinable-p' (by default).

Sounds good.

Regards,
  Dmitry

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] emacs: Make the part content available to `mm-inlinable-p'.
  2012-01-19  9:23           ` Dmitry Kurochkin
@ 2012-01-19  9:34             ` David Edmondson
  2012-01-19  9:37               ` Dmitry Kurochkin
                                 ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: David Edmondson @ 2012-01-19  9:34 UTC (permalink / raw)
  To: notmuch

The `mm-inlinable-p' function works better if it has access to the
data of the relevant part, so load that content before calling it.

Don't load the content for parts that the user has indicated no desire
to inline.

This fixes the display of attached image/jpeg parts, for example.
---

Updated as described in id:"cunboq06szv.fsf@hotblack-desiato.hh.sledj.net".

 emacs/notmuch-show.el |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 2df8d3b..7e9c9b4 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -330,14 +330,15 @@ current buffer, if possible."
     (with-temp-buffer
       (let* ((charset (plist-get part :content-charset))
 	     (handle (mm-make-handle (current-buffer) `(,content-type (charset . ,charset)))))
-	(if (and (mm-inlinable-p handle)
-		 (mm-inlined-p handle))
-	    (let ((content (notmuch-show-get-bodypart-content msg part nth)))
-	      (insert content)
-	      (set-buffer display-buffer)
-	      (mm-display-part handle)
-	      t)
-	  nil)))))
+	;; If the user wants the part inlined, insert the content and
+	;; test whether we are able to inline it (which includes both
+	;; capability and suitability tests).
+	(when (mm-inlined-p handle)
+	  (insert (notmuch-show-get-bodypart-content msg part nth))
+	  (when (mm-inlinable-p handle)
+	    (set-buffer display-buffer)
+	    (mm-display-part handle)
+	    t))))))
 
 (defvar notmuch-show-multipart/alternative-discouraged
   '(
-- 
1.7.8.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] emacs: Make the part content available to `mm-inlinable-p'.
  2012-01-19  9:34             ` [PATCH] emacs: Make the part content available to `mm-inlinable-p' David Edmondson
@ 2012-01-19  9:37               ` Dmitry Kurochkin
  2012-01-19 17:31               ` Aaron Ecay
                                 ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Dmitry Kurochkin @ 2012-01-19  9:37 UTC (permalink / raw)
  To: David Edmondson, notmuch

[and again with reply to all]

On Thu, 19 Jan 2012 09:34:07 +0000, David Edmondson <dme@dme.org> wrote:
> The `mm-inlinable-p' function works better if it has access to the
> data of the relevant part, so load that content before calling it.
> 
> Don't load the content for parts that the user has indicated no desire
> to inline.
> 
> This fixes the display of attached image/jpeg parts, for example.
> ---
> 
> Updated as described in id:"cunboq06szv.fsf@hotblack-desiato.hh.sledj.net".
> 

Looks good to me.

Regards,
  Dmitry

>  emacs/notmuch-show.el |   17 +++++++++--------
>  1 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 2df8d3b..7e9c9b4 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -330,14 +330,15 @@ current buffer, if possible."
>      (with-temp-buffer
>        (let* ((charset (plist-get part :content-charset))
>  	     (handle (mm-make-handle (current-buffer) `(,content-type (charset . ,charset)))))
> -	(if (and (mm-inlinable-p handle)
> -		 (mm-inlined-p handle))
> -	    (let ((content (notmuch-show-get-bodypart-content msg part nth)))
> -	      (insert content)
> -	      (set-buffer display-buffer)
> -	      (mm-display-part handle)
> -	      t)
> -	  nil)))))
> +	;; If the user wants the part inlined, insert the content and
> +	;; test whether we are able to inline it (which includes both
> +	;; capability and suitability tests).
> +	(when (mm-inlined-p handle)
> +	  (insert (notmuch-show-get-bodypart-content msg part nth))
> +	  (when (mm-inlinable-p handle)
> +	    (set-buffer display-buffer)
> +	    (mm-display-part handle)
> +	    t))))))
>  
>  (defvar notmuch-show-multipart/alternative-discouraged
>    '(
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] emacs: Make the part content available to `mm-inlinable-p'.
  2012-01-19  9:34             ` [PATCH] emacs: Make the part content available to `mm-inlinable-p' David Edmondson
  2012-01-19  9:37               ` Dmitry Kurochkin
@ 2012-01-19 17:31               ` Aaron Ecay
  2012-01-20 14:18               ` Tomi Ollila
                                 ` (2 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Aaron Ecay @ 2012-01-19 17:31 UTC (permalink / raw)
  To: David Edmondson, notmuch

On Thu, 19 Jan 2012 09:34:07 +0000, David Edmondson <dme@dme.org> wrote:
> The `mm-inlinable-p' function works better if it has access to the
> data of the relevant part, so load that content before calling it.
> 
> Don't load the content for parts that the user has indicated no desire
> to inline.
> 
> This fixes the display of attached image/jpeg parts, for example.

I had suggested the same approach in a message that only Dmitry got
(accursed new reply bindings!)  LGTM.

-- 
Aaron Ecay

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] emacs: Make the part content available to `mm-inlinable-p'.
  2012-01-19  9:34             ` [PATCH] emacs: Make the part content available to `mm-inlinable-p' David Edmondson
  2012-01-19  9:37               ` Dmitry Kurochkin
  2012-01-19 17:31               ` Aaron Ecay
@ 2012-01-20 14:18               ` Tomi Ollila
  2012-01-20 20:04               ` Jameson Graef Rollins
  2012-01-26 12:28               ` David Bremner
  4 siblings, 0 replies; 19+ messages in thread
From: Tomi Ollila @ 2012-01-20 14:18 UTC (permalink / raw)
  To: David Edmondson, notmuch

On Thu, 19 Jan 2012 09:34:07 +0000, David Edmondson <dme@dme.org> wrote:
> The `mm-inlinable-p' function works better if it has access to the
> data of the relevant part, so load that content before calling it.
> 
> Don't load the content for parts that the user has indicated no desire
> to inline.
> 
> This fixes the display of attached image/jpeg parts, for example.
> ---

LGTM.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] emacs: Make the part content available to `mm-inlinable-p'.
  2012-01-19  9:34             ` [PATCH] emacs: Make the part content available to `mm-inlinable-p' David Edmondson
                                 ` (2 preceding siblings ...)
  2012-01-20 14:18               ` Tomi Ollila
@ 2012-01-20 20:04               ` Jameson Graef Rollins
  2012-01-23  8:02                 ` David Edmondson
  2012-01-26 12:28               ` David Bremner
  4 siblings, 1 reply; 19+ messages in thread
From: Jameson Graef Rollins @ 2012-01-20 20:04 UTC (permalink / raw)
  To: David Edmondson, notmuch

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On Thu, 19 Jan 2012 09:34:07 +0000, David Edmondson <dme@dme.org> wrote:
> The `mm-inlinable-p' function works better if it has access to the
> data of the relevant part, so load that content before calling it.
> 
> Don't load the content for parts that the user has indicated no desire
> to inline.

So I'm a little confused here.  Is there some variable I need to set to
get inlinable stuff to display inline?  If so, I can't figure out what
it is.  I don't see anything relevant in notmuch config, and I'm not
finding anything in mm- either, although I'm not quite sure what I'm
looking for.

Currently I'm not getting any images displayed inline.  I used to get
some, but not all, but at the moment I'm just getting a little white box
with no image in it.  After applying this patch, I'm not getting any
attempt to inline images at all.  But I'm assuming there is some
variable I need to set that I haven't...

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] emacs: Make the part content available to the mm-inline* checks.
  2012-01-18 19:35         ` Austin Clements
  2012-01-18 19:59           ` Dmitry Kurochkin
@ 2012-01-20 20:13           ` Jameson Graef Rollins
  2012-01-23  8:00             ` David Edmondson
  1 sibling, 1 reply; 19+ messages in thread
From: Jameson Graef Rollins @ 2012-01-20 20:13 UTC (permalink / raw)
  To: Austin Clements, Dmitry Kurochkin; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On Wed, 18 Jan 2012 14:35:01 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Shouldn't we only be doing this for parts with inline (or not
> attachment) content-disposition?  That's cheap to check.  Or do we
> actually want things like image attachments to get inlined, despite
> their disposition?

This is a good question, actually.  Should we just always ignore the
disposition, and inline stuff if it's inlinable?  Should this be
configurable?

The whole disposition thing is actually pretty confused in general, I
think.  I'm not sure if people realize this but parts that are
disposition "attachment" are not indexed by notmuch, even if they're
imminently indexable.  This seems wrong to me, as I would like to have
as much of the message indexed as possible, regardless of disposition.
I'm not sure what the original motivation was there.

I point this out because there's a kind of schizophrenia related to
disposition handling in general, and it might be worthwhile to clarify
how we expect them to be handled, both in terms of indexing and display.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] emacs: Make the part content available to the mm-inline* checks.
  2012-01-20 20:13           ` Jameson Graef Rollins
@ 2012-01-23  8:00             ` David Edmondson
  0 siblings, 0 replies; 19+ messages in thread
From: David Edmondson @ 2012-01-23  8:00 UTC (permalink / raw)
  To: Jameson Graef Rollins, Austin Clements, Dmitry Kurochkin; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 750 bytes --]

On Fri, 20 Jan 2012 12:13:49 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Wed, 18 Jan 2012 14:35:01 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > Shouldn't we only be doing this for parts with inline (or not
> > attachment) content-disposition?  That's cheap to check.  Or do we
> > actually want things like image attachments to get inlined, despite
> > their disposition?
> 
> This is a good question, actually.  Should we just always ignore the
> disposition, and inline stuff if it's inlinable?  Should this be
> configurable?

We shouldn't inline things unless the content disposition is 'inline'.

I'll work on a fix for this, which will have nothing to do with whether
or not things are indexed.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] emacs: Make the part content available to `mm-inlinable-p'.
  2012-01-20 20:04               ` Jameson Graef Rollins
@ 2012-01-23  8:02                 ` David Edmondson
  0 siblings, 0 replies; 19+ messages in thread
From: David Edmondson @ 2012-01-23  8:02 UTC (permalink / raw)
  To: Jameson Graef Rollins; +Cc: notmuch

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

On Fri, 20 Jan 2012 12:04:27 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Thu, 19 Jan 2012 09:34:07 +0000, David Edmondson <dme@dme.org> wrote:
> > The `mm-inlinable-p' function works better if it has access to the
> > data of the relevant part, so load that content before calling it.
> > 
> > Don't load the content for parts that the user has indicated no desire
> > to inline.
> 
> So I'm a little confused here.  Is there some variable I need to set to
> get inlinable stuff to display inline?  If so, I can't figure out what
> it is.  I don't see anything relevant in notmuch config, and I'm not
> finding anything in mm- either, although I'm not quite sure what I'm
> looking for.
> 
> Currently I'm not getting any images displayed inline.  I used to get
> some, but not all, but at the moment I'm just getting a little white box
> with no image in it.  After applying this patch, I'm not getting any
> attempt to inline images at all.  But I'm assuming there is some
> variable I need to set that I haven't...

Are you talking about:
    - straightforward image/* attachments,
    - images in HTML which are part of a multipart/related ('cid:'
      images),
    - images in HTML which are referenced using HTTP (or FTP or ... I
      suppose).
?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] emacs: Make the part content available to `mm-inlinable-p'.
  2012-01-19  9:34             ` [PATCH] emacs: Make the part content available to `mm-inlinable-p' David Edmondson
                                 ` (3 preceding siblings ...)
  2012-01-20 20:04               ` Jameson Graef Rollins
@ 2012-01-26 12:28               ` David Bremner
  4 siblings, 0 replies; 19+ messages in thread
From: David Bremner @ 2012-01-26 12:28 UTC (permalink / raw)
  To: David Edmondson, notmuch

On Thu, 19 Jan 2012 09:34:07 +0000, David Edmondson <dme@dme.org> wrote:
> The `mm-inlinable-p' function works better if it has access to the
> data of the relevant part, so load that content before calling it.

pushed

d

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2012-01-26 12:28 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-18 17:33 [PATCH] emacs: Make the part content available to the mm-inline* checks David Edmondson
2012-01-18 17:35 ` David Edmondson
2012-01-18 17:39 ` [PATCH v2] " David Edmondson
2012-01-18 18:04   ` Dmitry Kurochkin
2012-01-18 18:30     ` David Edmondson
2012-01-18 19:00       ` Dmitry Kurochkin
2012-01-18 19:35         ` Austin Clements
2012-01-18 19:59           ` Dmitry Kurochkin
2012-01-20 20:13           ` Jameson Graef Rollins
2012-01-23  8:00             ` David Edmondson
2012-01-19  7:39         ` David Edmondson
2012-01-19  9:23           ` Dmitry Kurochkin
2012-01-19  9:34             ` [PATCH] emacs: Make the part content available to `mm-inlinable-p' David Edmondson
2012-01-19  9:37               ` Dmitry Kurochkin
2012-01-19 17:31               ` Aaron Ecay
2012-01-20 14:18               ` Tomi Ollila
2012-01-20 20:04               ` Jameson Graef Rollins
2012-01-23  8:02                 ` David Edmondson
2012-01-26 12:28               ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).