bug#65872: [PATCH] Fix interactive display of MIME parts

Davide Masserut [31/Jul  6:14pm +02] wrote:
> The problem is confined to the interactive selection:
> `completing-read' accepts the mixed collection but returns the
> candidate as a string.

Thanks, this is clearer to me now.

> `mailcap-mime-info' may return viewer functions as symbols as well as
> external commands as strings.  However, `completing-read' returns both
> as strings, causing `mm-display-external' to treat a selected Lisp
> viewer as a shell command.

We prefer not to include this sort of commentary in commit messages.  If
it's important to say, it's better in a code comment.

> diff --git a/lisp/gnus/mm-decode.el b/lisp/gnus/mm-decode.el
> index 717c8a022ef..910246f46de 100644
> --- a/lisp/gnus/mm-decode.el
> +++ b/lisp/gnus/mm-decode.el
> @@ -1503,13 +1503,19 @@ mm-interactively-view-part
>  	 (methods
>  	  (mapcar (lambda (i) (cdr (assoc 'viewer i)))
>  		  (mailcap-mime-info type 'all)))
> -	 (method (let ((minibuffer-local-completion-map
> -			mm-viewer-completion-map))
> -		   (completing-read "Viewer: " methods))))
> +	 (method-name
> +	  (let ((minibuffer-local-completion-map
> +		 mm-viewer-completion-map))
> +	    (completing-read "Viewer: " methods)))
> +	 ;; `completing-read' converts symbols to strings.  Recover the
> +	 ;; corresponding object from the original collection.
> +	 (method (or (cl-find method-name methods :test #'string=)
> +		     method-name)))

I think you're deliberately using the property of string= that it works
on symbols, here?  Could the case where METHODS contains both "foo" and
`foo' cause problems?

I think the more idiomatic way to handle this is to make COLLECTION an
alist, something supported by completing-read for this sort of case.

-- 
Sean Whitton