PATCH: Unbreak `mailcap-mailcap-entry-passes-test'

PATCH: Unbreak `mailcap-mailcap-entry-passes-test'

[Michael Sperber]

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


I had to apply to attached patch to make mailcap work on XEmacs.  I'm
puzzled by this, as I think it should break mailcap for just about
everyone.  On the other hand, I don't see how the code (without) the
patch could work as advertised.

So if somebody could review (and maybe apply) it, that - again - would
be much appreciated.

-- 
Regards,
Mike

[-- Attachment #2: Type: text/plain, Size: 2655 bytes --]

commit 44ac8cc38c99a6f88fc1ea9d065c8971ac39e36b
Author: Mike Sperber <sperber@deinprogramm.de>
Date:   Tue Oct 20 11:06:49 2015 +0200

    	* mailcap.el (mailcap-mailcap-entry-passes-test): Actually return
    	non-nil if no test clause is present, as documented.

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 3e2f0e1..56618b6 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,5 +1,10 @@
 2015-10-20  Michael Sperber  <mike@xemacs.org>
 
+	* mailcap.el (mailcap-mailcap-entry-passes-test): Actually return
+	non-nil if no test clause is present, as documented.
+					   
+2015-10-20  Michael Sperber  <mike@xemacs.org>
+
 	* mailcap.el (mailcap-mime-data): Conditonalize `doc-view-mode', which
 	does not exist on XEmacs.
 
diff --git a/lisp/mailcap.el b/lisp/mailcap.el
index a3348c6..cdd20d1 100644
--- a/lisp/mailcap.el
+++ b/lisp/mailcap.el
@@ -560,26 +560,28 @@ MAILCAPS if set; otherwise (on Unix) use the path from RFC 1524, plus
 Also return non-nil if no test clause is present."
   (let ((test (assq 'test info))	; The test clause
 	status)
-    (setq status (and test (split-string (cdr test) " ")))
-    (if (and (or (assoc "needsterm" info)
-		 (assoc "needsterminal" info)
-		 (assoc "needsx11" info))
-	     (not (getenv "DISPLAY")))
-	(setq status nil)
-      (cond
-       ((and (equal (nth 0 status) "test")
-	     (equal (nth 1 status) "-n")
-	     (or (equal (nth 2 status) "$DISPLAY")
-		 (equal (nth 2 status) "\"$DISPLAY\"")))
-	(setq status (if (getenv "DISPLAY") t nil)))
-       ((and (equal (nth 0 status) "test")
-	     (equal (nth 1 status) "-z")
-	     (or (equal (nth 2 status) "$DISPLAY")
-		 (equal (nth 2 status) "\"$DISPLAY\"")))
-	(setq status (if (getenv "DISPLAY") nil t)))
-       (test nil)
-       (t nil)))
-    (and test (listp test) (setcdr test status))))
+    (or (not test)
+	(progn
+	  (setq status (and test (split-string (cdr test) " ")))
+	  (if (and (or (assoc "needsterm" info)
+		       (assoc "needsterminal" info)
+		       (assoc "needsx11" info))
+		   (not (getenv "DISPLAY")))
+	      (setq status nil)
+	    (cond
+	     ((and (equal (nth 0 status) "test")
+		   (equal (nth 1 status) "-n")
+		   (or (equal (nth 2 status) "$DISPLAY")
+		       (equal (nth 2 status) "\"$DISPLAY\"")))
+	      (setq status (if (getenv "DISPLAY") t nil)))
+	     ((and (equal (nth 0 status) "test")
+		   (equal (nth 1 status) "-z")
+		   (or (equal (nth 2 status) "$DISPLAY")
+		       (equal (nth 2 status) "\"$DISPLAY\"")))
+	      (setq status (if (getenv "DISPLAY") nil t)))
+	     (test nil)
+	     (t nil)))
+	  (and test (listp test) (setcdr test status))))))
 
 ;;;
 ;;; The action routines.

Re: PATCH: Unbreak `mailcap-mailcap-entry-passes-test'

[Katsumi Yamaoka]

On Tue, 20 Oct 2015 11:10:55 +0200, Michael Sperber wrote:
> I had to apply to attached patch to make mailcap work on XEmacs.  I'm
> puzzled by this, as I think it should break mailcap for just about
> everyone.  On the other hand, I don't see how the code (without) the
> patch could work as advertised.

> So if somebody could review (and maybe apply) it, that - again - would
> be much appreciated.

This is what I grasped of how mailcap.el does:

What is passed to mailcap-mailcap-entry-passes-test is an entry
of a mailcap file (user's or system's) like this:

image/*; xv %s; test=test -n "$DISPLAY"

This entry is parsed by mailcap-parse-mailcap into:

((viewer . "xv %s") (type . "image/*") (test . "test -n \"$DISPLAY\""))

It is passed to mailcap-mailcap-entry-passes-test as an argument,
and the result is added to mailcap-mime-data.  Where the result is

((viewer . "xv %s") (type . "image/*") (test . t))

if $DISPLAY is not empty, otherwise:

((viewer . "xv %s") (type . "image/*") (test))

If your patch is applied, the `test' token will not be updated
whatever $DISPLAY is.

Regards,

Re: PATCH: Unbreak `mailcap-mailcap-entry-passes-test'

[Michael Sperber]

Thanks for the quick action!

Katsumi Yamaoka <yamaoka@jpl.org> writes:

> This is what I grasped of how mailcap.el does:
>
> What is passed to mailcap-mailcap-entry-passes-test is an entry
> of a mailcap file (user's or system's) like this:
>
> image/*; xv %s; test=test -n "$DISPLAY"
>
> This entry is parsed by mailcap-parse-mailcap into:
>
> ((viewer . "xv %s") (type . "image/*") (test . "test -n \"$DISPLAY\""))
>
> It is passed to mailcap-mailcap-entry-passes-test as an argument,
> and the result is added to mailcap-mime-data.  Where the result is
>
> ((viewer . "xv %s") (type . "image/*") (test . t))
>
> if $DISPLAY is not empty, otherwise:
>
> ((viewer . "xv %s") (type . "image/*") (test))

Right.  However, many typical mailcap entries will not have a test
clause.

> If your patch is applied, the `test' token will not be updated
> whatever $DISPLAY is.

I don't think so:

If there's a test clause present, the same code as before runs -
mutating the cdr as before.

If not, the function simply returns t, as documented.  (In that case,
there's nothing to mutate.)

-- 
Regards,
Mike

Re: PATCH: Unbreak `mailcap-mailcap-entry-passes-test'

[Katsumi Yamaoka]

Michael Sperber <sperber@deinprogramm.de> wrote:
> Right.  However, many typical mailcap entries will not have a test
> clause.

Yes.  Moreover mailcap-mailcap-entry-passes-test seems to
support only `test -n $DISPLAY' and `test -z $DISPLAY'.

>> If your patch is applied, the `test' token will not be updated
>> whatever $DISPLAY is.

> I don't think so:

> If there's a test clause present, the same code as before runs -
> mutating the cdr as before.

> If not, the function simply returns t, as documented.  (In that case,
> there's nothing to mutate.)

I did misread the patched code, sorry.

> +    (or (not test)
> +	(progn
> 	  ...))

In the case `test' is nil, there is no `test' to be updated.
So, the patch is harmless, but it isn't necessarily worthwhile
except that it makes it a bit fast, I think.  In addition,

> * mailcap.el (mailcap-mailcap-entry-passes-test): Actually return
> non-nil if no test clause is present, as documented.

Actually the return value of the function is not used in at least
Gnus Lisp modules.

Regards,

Re: PATCH: Unbreak `mailcap-mailcap-entry-passes-test'

[Michael Sperber]

I appreciate your looking at this again!

Katsumi Yamaoka <yamaoka@jpl.org> writes:

> In the case `test' is nil, there is no `test' to be updated.
> So, the patch is harmless, but it isn't necessarily worthwhile
> except that it makes it a bit fast, I think.

You're right.  Somehow I convinced myself that my mailcap entries didn't
get added.  Sorry for the false alarm!

-- 
Regards,
Mike