[Patch/Question] Likely bugs in mailcap docs and mailcap-view-file

[Tassilo Horn]

Hi all,

I wanted to use `mailcap-view-file' in some package (mu4e) in order to
view some attachment using some appropriate viewer.  I've tested with
some pdf attachment which triggered an error because I use pdf-tools
which adds an entry to `mailcap-mime-data' where the viewer is the
symbol `pdf-view-mode' (a major-mode) whereas `mailcap-view-file'
currently only handles the "viewer is a string naming some executable"
case.

The default value of `mailcap-mime-data' has tons of viewers being
major-mode symbols, so that can't be wrong.  It's docstring also says:

    Where VIEWERINFO specifies how the content-type is viewed.  Can be a
    string, in which case it is run through a shell, with appropriate
    parameters, or a symbol, in which case the symbol is ‘funcall’ed if
    and only if it exists as a function, with the buffer as an argument.

Well, that's strange.  All symbol-valued VIEWERINFOs are functions of
zero args, mostly major-modes.  They can't get the buffer (which
buffer?!) as an argument.

So I propose the attached patch which teaches `mailcap-view-file' the
symbol/function-valued viewer case and adjust the docs to match what's
already there.

diff --git a/lisp/net/mailcap.el b/lisp/net/mailcap.el
index bf3c8edd1e..151c6e0509 100644
--- a/lisp/net/mailcap.el
+++ b/lisp/net/mailcap.el
@@ -319,8 +319,9 @@ mailcap-mime-data
 
 Where VIEWERINFO specifies how the content-type is viewed.  Can be
 a string, in which case it is run through a shell, with appropriate
-parameters, or a symbol, in which case the symbol is `funcall'ed if
-and only if it exists as a function, with the buffer as an argument.
+parameters, or a symbol, in which case the symbol must name a function
+of zero arguments which is called in a buffer holding the MIME part's
+content.
 
 TESTINFO is a test for the viewer's applicability, or nil.  If nil, it
 means the viewer is always valid.  If it is a Lisp function, it is
@@ -1174,34 +1175,45 @@ mailcap-view-file
   (mailcap-parse-mailcaps)
   (let ((command (mailcap-mime-info
                   (mailcap-extension-to-mime (file-name-extension file)))))
-    (unless command
-      (error "No viewer for %s" (file-name-extension file)))
-    ;; Remove quotes around the file name - we'll use shell-quote-argument.
-    (while (string-match "['\"]%s['\"]" command)
-      (setq command (replace-match "%s" t t command)))
-    (setq command (replace-regexp-in-string
-                  "%s"
-                  (shell-quote-argument (convert-standard-filename file))
-                  command
-                  nil t))
-    ;; Handlers such as "gio open" and kde-open5 start viewer in background
-    ;; and exit immediately.  Avoid `start-process' since it assumes
-    ;; :connection-type `pty' and kills children processes with SIGHUP
-    ;; when temporary terminal session is finished (Bug#44824).
-    ;; An alternative is `process-connection-type' let-bound to nil for
-    ;; `start-process-shell-command' call (with no chance to report failure).
-    (make-process
-     :name "mailcap-view-file"
-     :connection-type 'pipe
-     :buffer nil ; "*Messages*" may be suitable for debugging
-     :sentinel (lambda (proc event)
-                 (when (and (memq (process-status proc) '(exit signal))
-                            (/= (process-exit-status proc) 0))
-                   (message
-                    "Command %s: %s."
-                    (mapconcat #'identity (process-command proc) " ")
-                    (substring event 0 -1))))
-     :command (list shell-file-name shell-command-switch command))))
+    (if (functionp command)
+        ;; command is a viewer function (a mode) expecting the file
+        ;; contents to be in the current buffer.
+        (let ((buf (generate-new-buffer (file-name-nondirectory file))))
+          (set-buffer buf)
+          (insert-file-contents file)
+          (setq buffer-file-name file)
+          (funcall command)
+          (set-buffer-modified-p nil)
+          (pop-to-buffer buf))
+      ;; command is a program to run with file as an argument.
+      (unless command
+        (error "No viewer for %s" (file-name-extension file)))
+      ;; Remove quotes around the file name - we'll use shell-quote-argument.
+      (while (string-match "['\"]%s['\"]" command)
+        (setq command (replace-match "%s" t t command)))
+      (setq command (replace-regexp-in-string
+                    "%s"
+                    (shell-quote-argument (convert-standard-filename file))
+                    command
+                    nil t))
+      ;; Handlers such as "gio open" and kde-open5 start viewer in background
+      ;; and exit immediately.  Avoid `start-process' since it assumes
+      ;; :connection-type `pty' and kills children processes with SIGHUP
+      ;; when temporary terminal session is finished (Bug#44824).
+      ;; An alternative is `process-connection-type' let-bound to nil for
+      ;; `start-process-shell-command' call (with no chance to report failure).
+      (make-process
+       :name "mailcap-view-file"
+       :connection-type 'pipe
+       :buffer nil ; "*Messages*" may be suitable for debugging
+       :sentinel (lambda (proc event)
+                   (when (and (memq (process-status proc) '(exit signal))
+                              (/= (process-exit-status proc) 0))
+                     (message
+                      "Command %s: %s."
+                      (mapconcat #'identity (process-command proc) " ")
+                      (substring event 0 -1))))
+       :command (list shell-file-name shell-command-switch command)))))
 
 (provide 'mailcap)
 

Does anyone see a problem with that?  I'm not very familiar with that
code.

Bye,
Tassilo