Re: Displaying MIME parts internally/externally in Gnus

Re: Displaying MIME parts internally/externally in Gnus

[Reiner Steib]

On Mon, Nov 21 2005, Stefan Monnier wrote:

| To: emacs-devel@gnu.org, gnus@gnus.org

Probably you meant ding@gnus.org?

> Many attachments come with poor content-type description, in which case Gnus
> typically doesn't know what to do with it.  When doing "view externally",
> such attachments are actually saved.  I don't know about you, but I don't
> consider "saving" as a sort of "viewing" activity.  If I had wanted to save
> the attachment I'd have used something more like, say, the "Save"
> command instead, don't you think?

> Similar problems happen for "view internally".  So I suggest the patch below
> which tries harder to show the attachment internally or externally: if the
> content-type can't be displayed as requested, it asks the user what kind of
> content-type to use instead.
>
> A minor variant of it has been mildly tested (and only on Emacs-CVS), and
> it's based on a very shallow understanding of the code, so it's probably
> not perfect.
>
> If there's no objection I'll install it, otherwise please tell me what's
> wrong with it.

I don't see how you use the predicate `PRED'.

Your patch may conflict with the one I've installed in the trunk to
improve `gnus-mime-view-part-as-type' (see below[1] and
http://thread.gmane.org/v9hdbnepee.fsf%40marauder.physik.uni-ulm.de).

Does you patch address a similar scenario?  (Sorry, I didn't have time
to check it myself.)

Bye, Reiner.

[1]
2005-10-13  Reiner Steib  <Reiner.Steib@gmx.de>
[...]
	* gnus-art.el (gnus-mime-view-part-as-type-internal): Try to fetch
	`filename' from Content-Disposition if Content-Type doesn't
	provide `name'.
	(gnus-mime-view-part-as-type): Set default instead of
	initial-input.

--8<---------------cut here---------------start------------->8---
--- gnus-art.el	12 Oct 2005 12:50:14 -0000	7.129
+++ gnus-art.el	13 Oct 2005 13:26:03 -0000	7.130
@@ -4317,9 +4317,12 @@
 
 (defun gnus-mime-view-part-as-type-internal ()
   (gnus-article-check-buffer)
-  (let* ((name (mail-content-type-get
-		(mm-handle-type (get-text-property (point) 'gnus-data))
-		'name))
+  (let* ((handle (get-text-property (point) 'gnus-data))
+	 (name (or
+		;; Content-Type: foo/bar; name=...
+		(mail-content-type-get (mm-handle-type handle) 'name)
+		;; Content-Disposition: attachment; filename=...
+		(cdr (assq 'filename (cdr (mm-handle-disposition handle))))))
 	 (def-type (and name (mm-default-file-encoding name))))
     (and def-type (cons def-type 0))))
 
@@ -4327,11 +4330,14 @@
   "Choose a MIME media type, and view the part as such."
   (interactive)
   (unless mime-type
-    (setq mime-type (completing-read
-		     "View as MIME type: "
-		     (mapcar #'list (mailcap-mime-types))
-		     nil nil
-		     (gnus-mime-view-part-as-type-internal))))
+    (setq mime-type
+	  (let ((default (gnus-mime-view-part-as-type-internal)))
+	    (completing-read
+	     (format "View as MIME type (default %s): "
+		     (car default))
+	     (mapcar #'list (mailcap-mime-types))
+	     nil nil nil nil
+	     (car default)))))
   (gnus-article-check-buffer)
   (let ((handle (get-text-property (point) 'gnus-data)))
     (when handle
--8<---------------cut here---------------end--------------->8---
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: Displaying MIME parts internally/externally in Gnus

[Stefan Monnier]

> | To: emacs-devel@gnu.org, gnus@gnus.org
> Probably you meant ding@gnus.org?

Could be, although making an alias would make sense, wouldn't it?
I mean, when was the last time you've referred to Gnus as "ding"?

> I don't see how you use the predicate `PRED'.

It uses it to constrain the list of completions such that when completing
the content-type.  E.g. from "view externally", it constrains the list to
the content-types that Gnus knows how to show externally.

> Your patch may conflict with the one I've installed in the trunk to
> improve `gnus-mime-view-part-as-type' (see below[1] and
> http://thread.gmane.org/v9hdbnepee.fsf%40marauder.physik.uni-ulm.de).

Could be.  I only use the Emacs-CVS version of the code.

> Does you patch address a similar scenario?  (Sorry, I didn't have time
> to check it myself.)

Not that I can see, no.


        Stefan

Re: Displaying MIME parts internally/externally in Gnus

[Reiner Steib]

On Mon, Apr 17 2006, Lars Magne Ingebrigtsen wrote:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> Well, I work within the Emacs-CVS code, so my patch was made relative to
>> Gnus-5.10, but feel free to only install it in CVS Gnus.
>>
>> I think of it as a bug fix, but I can see how you may disagree, especially
>> if you're really close to a release.
>
> Looking over the code in CVS Gnus more carefully, I see that the code
> for `gnus-mime-view-part-as-type' has changed substantially.  It may
> not actually be necessary any more, but I might be misreading it.
>
> Anybody?

My patch[1] addressed only `gnus-mime-view-part-as-type' whereas
Stefan's patch cares about `gnus-mime-view-part-internally' and
`gnus-mime-view-part-externally'.

I will apply Stefan's changes in the trunk now.  Unless you object, I
will apply both to v5-10 / Emacs-CVS as well.

Bye, Reiner.


[1]
,----
| 2005-10-13  Reiner Steib  <Reiner.Steib@gmx.de>
| [...]
| 	* gnus-art.el (gnus-mime-view-part-as-type-internal): Try to fetch
| 	`filename' from Content-Disposition if Content-Type doesn't
| 	provide `name'.
| 	(gnus-mime-view-part-as-type): Set default instead of
| 	initial-input.
`----
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: Displaying MIME parts internally/externally in Gnus

[Lars Magne Ingebrigtsen]

Reiner Steib <reinersteib+from-uce@imap.cc> writes:

> I will apply Stefan's changes in the trunk now.  Unless you object, I
> will apply both to v5-10 / Emacs-CVS as well.

I guess this counts as a bug, so applying it to 5.10 makes sense.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Displaying MIME parts internally/externally in Gnus

[Katsumi Yamaoka]

>>>>> In <v97j532omg.fsf@marauder.physik.uni-ulm.de>
>>>>>	Reiner Steib <reinersteib+from-uce@imap.cc> wrote:

> I will apply Stefan's changes in the trunk now.  Unless you object, I
> will apply both to v5-10 / Emacs-CVS as well.

Well, how should we change the `mm-inlinable-p' function?

In gnus-mime-view-part-internally:
gnus-art.el:4740:30:Warning: mm-inlinable-p called with 2 arguments, but
    accepts only 1

Re: Displaying MIME parts internally/externally in Gnus

[Reiner Steib]

On Thu, May 04 2006, Katsumi Yamaoka wrote:

>>>>>>	Reiner Steib <reinersteib+from-uce@imap.cc> wrote:
>
>> I will apply Stefan's changes in the trunk now.  Unless you object, I
>> will apply both to v5-10 / Emacs-CVS as well.
>
> Well, how should we change the `mm-inlinable-p' function?
>
> In gnus-mime-view-part-internally:
> gnus-art.el:4740:30:Warning: mm-inlinable-p called with 2 arguments, but
>     accepts only 1

It is this hunk in Stefan's patch:

--8<---------------cut here---------------start------------->8---
@@ -4467,13 +4475,16 @@
 	 (mm-inline-large-images t)
 	 (mail-parse-charset gnus-newsgroup-charset)
 	 (mail-parse-ignored-charsets
-	  (save-excursion (set-buffer gnus-summary-buffer)
+          (with-current-buffer gnus-summary-buffer
 			  gnus-newsgroup-ignored-charsets))
 	 (inhibit-read-only t))
+    (if (not (mm-inlinable-p handle))
+        (gnus-mime-view-part-as-type
+         nil (lambda (type) (mm-inlinable-p handle type)))
     (when handle
       (if (mm-handle-undisplayer handle)
 	  (mm-remove-part handle)
-	(mm-display-part handle)))))
+          (mm-display-part handle))))))

 (defun gnus-mime-action-on-part (&optional action)
   "Do something with the MIME attachment at \(point\)."
--8<---------------cut here---------------end--------------->8---

In `mm-inlinable-p', TYPE is the value of (mm-handle-media-type handle):

--8<---------------cut here---------------start------------->8---
(defun mm-inlinable-p (handle)
  "Say whether HANDLE can be displayed inline."
  (let ((alist mm-inline-media-tests)
	(type (mm-handle-media-type handle))
	test)
    (while alist
      (when (string-match (caar alist) type)
	(setq test (caddar alist)
	      alist nil)
	(setq test (funcall test handle)))
      (pop alist))
    test))
--8<---------------cut here---------------end--------------->8---

Stefan, maybe you forgot to send a patch for `mm-decode.el' as well?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: Displaying MIME parts internally/externally in Gnus

[Stefan Monnier]

> In `mm-inlinable-p', TYPE is the value of (mm-handle-media-type handle):
[...]
> Stefan, maybe you forgot to send a patch for `mm-decode.el' as well?

Oops, looks like you're right.  The patch is the obvious one.
Sorry 'bout that.


        Stefan


--- orig/lisp/gnus/mm-decode.el
+++ mod/lisp/gnus/mm-decode.el
@@ -562,7 +562,7 @@
 	   description)
 	(setq type (split-string (car ctl) "/"))
 	(setq subtype (cadr type)
-	      type (pop type))
+	      type (car type))
 	(setq
 	 result
 	 (cond
@@ -641,16 +641,15 @@
 
 (defun mm-copy-to-buffer ()
   "Copy the contents of the current buffer to a fresh buffer."
-  (save-excursion
     (let ((obuf (current-buffer))
 	  beg)
       (goto-char (point-min))
       (search-forward-regexp "^\n" nil t)
       (setq beg (point))
-      (set-buffer
+    (with-current-buffer
        ;; Preserve the data's unibyteness (for url-insert-file-contents).
        (let ((default-enable-multibyte-characters (mm-multibyte-p)))
-	 (generate-new-buffer " *mm*")))
+          (generate-new-buffer " *mm*"))
       (insert-buffer-substring obuf beg)
       (current-buffer))))
 
@@ -701,7 +701,8 @@
 		  (forward-line 1)
 		  (mm-insert-inline handle (mm-get-part handle))
 		  'inline)
-	      (if (and method ;; If nil, we always use "save".
+	      (setq external
+                    (and method ;; If nil, we always use "save".
 		       (stringp method) ;; 'mailcap-save-binary-file
 		       (or (eq mm-enable-external t)
 			   (and (eq mm-enable-external 'ask)
@@ -714,9 +715,7 @@
 				      (concat
 				       " \"" (format method filename) "\"")
 				    "")
-				  "? ")))))
-		  (setq external t)
-		(setq external nil))
+                                    "? "))))))
 	      (if external
 		  (mm-display-external
 		   handle (or method 'mailcap-save-binary-file))
@@ -1019,10 +1017,10 @@
 	      methods nil)))
     result))
 
-(defun mm-inlinable-p (handle)
+(defun mm-inlinable-p (handle &optional type)
   "Say whether HANDLE can be displayed inline."
+  (unless type (setq type (mm-handle-media-type handle)))
   (let ((alist mm-inline-media-tests)
-	(type (mm-handle-media-type handle))
 	test)
     (while alist
       (when (string-match (caar alist) type)

Re: Displaying MIME parts internally/externally in Gnus

[Reiner Steib]

On Thu, May 04 2006, Stefan Monnier wrote:

>> In `mm-inlinable-p', TYPE is the value of (mm-handle-media-type handle):
> [...]
>> Stefan, maybe you forgot to send a patch for `mm-decode.el' as well?
>
> Oops, looks like you're right.  The patch is the obvious one.

Thanks.  Could you provide a ChangeLog entry, please?  AFAICS, the
first two hunks are unrelated (maybe the ChangeLog entry will
clarify).

For the `gnus-art.el' changes, I've added the following.  Feel free to
suggest a better entry.

--8<---------------cut here---------------start------------->8---
2006-05-03  Stefan Monnier  <monnier@iro.umontreal.ca>

	* gnus-art.el (gnus-mime-view-part-as-type): Add optional PRED
	argument.
	(gnus-mime-view-part-externally, gnus-mime-view-part-internally):
	Try harder to show the attachment internally or externally using
	gnus-mime-view-part-as-type.
--8<---------------cut here---------------end--------------->8---


> @@ -1019,10 +1017,10 @@
>  	      methods nil)))
>      result))
>
> -(defun mm-inlinable-p (handle)
> +(defun mm-inlinable-p (handle &optional type)
>    "Say whether HANDLE can be displayed inline."
> +  (unless type (setq type (mm-handle-media-type handle)))
>    (let ((alist mm-inline-media-tests)
> -	(type (mm-handle-media-type handle))
>  	test)
>      (while alist
>        (when (string-match (caar alist) type)

IMHO, the doc string should explain the use of TYPE, i.e. if TYPE is
given, HANDLE is ignored.

BTW, I did some test with *.pdf and *.png files attached as
application/octet-stream.  Using `e', I get "View as MIME type
(default image/png)" but TAB doesn't offer me any completion.

Maybe an addition to (info "(gnus)Using MIME") for `e' and `E' would
be useful.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: Displaying MIME parts internally/externally in Gnus

[Stefan Monnier]

>>> In `mm-inlinable-p', TYPE is the value of (mm-handle-media-type handle):
>> [...]
>>> Stefan, maybe you forgot to send a patch for `mm-decode.el' as well?
>> Oops, looks like you're right.  The patch is the obvious one.
> Thanks.  Could you provide a ChangeLog entry, please?

OK:

2006-05-04  Stefan Monnier  <monnier@iro.umontreal.ca>

	* mm-decode.el (mm-dissect-buffer): Remove spurious double assignment.
	(mm-copy-to-buffer): Use with-current-buffer.
	(mm-display-part): Simplify.
	(mm-inlinable-p): Add optional arg `type'.

> AFAICS, the first two hunks are unrelated (maybe the ChangeLog entry will
> clarify).

Indeed, they're just "clean up".

> For the `gnus-art.el' changes, I've added the following.  Feel free to
> suggest a better entry.

> --8<---------------cut here---------------start------------->8---
> 2006-05-03  Stefan Monnier  <monnier@iro.umontreal.ca>

> 	* gnus-art.el (gnus-mime-view-part-as-type): Add optional PRED
> 	argument.
> 	(gnus-mime-view-part-externally, gnus-mime-view-part-internally):
> 	Try harder to show the attachment internally or externally using
> 	gnus-mime-view-part-as-type.
> --8<---------------cut here---------------end--------------->8---

Sounds good.

>> @@ -1019,10 +1017,10 @@
>> methods nil)))
>> result))
>> 
>> -(defun mm-inlinable-p (handle)
>> +(defun mm-inlinable-p (handle &optional type)
>> "Say whether HANDLE can be displayed inline."
>> +  (unless type (setq type (mm-handle-media-type handle)))
>> (let ((alist mm-inline-media-tests)
>> -	(type (mm-handle-media-type handle))
>> test)
>> (while alist
>> (when (string-match (caar alist) type)

> IMHO, the doc string should explain the use of TYPE, i.e. if TYPE is
> given, HANDLE is ignored.

Yes, sorry.  I just sent my "current diff" without massaging it.
The docstring should also say:

TYPE is the mime-type of the object; it defaults to the one given in HANDLE.

And AFAICT the `handle' is not ignored: it's passed to the inline-media
test function.

> BTW, I did some test with *.pdf and *.png files attached as
> application/octet-stream.  Using `e', I get "View as MIME type
> (default image/png)" but TAB doesn't offer me any completion.

Hmm.. can't test it right now, but it's worked for me.  Note that part of
the reason why I originally sent my patch rather than just install it
(other than the fact that it needed approval) is because I have no idea
whether it's doing things The Right Way.  I have very little familiarity
with Gnus (and MM) code.


        Stefan

Re: Displaying MIME parts internally/externally in Gnus

[Reiner Steib]

On Thu, May 04 2006, Stefan Monnier wrote:

> 2006-05-04  Stefan Monnier  <monnier@iro.umontreal.ca>
>
> 	* mm-decode.el (mm-dissect-buffer): Remove spurious double assignment.
> 	(mm-copy-to-buffer): Use with-current-buffer.
> 	(mm-display-part): Simplify.
> 	(mm-inlinable-p): Add optional arg `type'.

Applied to trunk and v5-10 along with the related patches to
`gnus-art.el'.  (Synced to Emacs CVS as well in the meantime.)

> The docstring should also say:
>
> TYPE is the mime-type of the object; it defaults to the one given in HANDLE.

Added.

> And AFAICT the `handle' is not ignored: it's passed to the inline-media
> test function.

Yes, I missed this.

>> BTW, I did some test with *.pdf and *.png files attached as
>> application/octet-stream.  Using `e', I get "View as MIME type
>> (default image/png)" but TAB doesn't offer me any completion.
>
> Hmm.. can't test it right now, but it's worked for me.

I've posted a test article in gmane.test:
<news:v9veslyd4s.fsf@marauder.physik.uni-ulm.de>
<nntp://news.gmane.org/gmane.test/2916>

I get "No match" after `e TAB'.  `t TAB' offers all MIME types.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/