bug#65468: 29.1; describe-theme fails to describe some themes not loaded

[Mauro Aranda <maurooaranda@gmail.com>]

Thierry Volpiatto <thievol@posteo.net> writes:

 > This because `describe-theme-1` is not looping in buffer to find
 > `deftheme` definition.

It is a convention that the first form should be a call to deftheme.
But it seems built-in themes have deviated from that convention.  And
other themes might as well, I don't know.

 >
 > Try (describe-theme 'leuven) to reproduce (if not already loaded of 
course).
 >

At least for the leuven themes, it should be easy to make them follow
the convention.

 > This patch fixes it:
 >
 > diff --git a/lisp/cus-theme.el b/lisp/cus-theme.el
 > index 5d3f2585976..3640d1ec329 100644
 > --- a/lisp/cus-theme.el
 > +++ b/lisp/cus-theme.el
 > @@ -513,13 +513,15 @@ It includes all faces in list FACES."
 >        ;; Attempt to grab the theme documentation
 >        (when fn
 >  	(with-temp-buffer
 > -	  (insert-file-contents fn)
 > -	  (let ((sexp (let ((read-circle nil))
 > -			(condition-case nil
 > -			    (read (current-buffer))
 > -			  (end-of-file nil)))))
 > -            (and (eq (car-safe sexp) 'deftheme)
 > -		 (setq doc (nth 2 sexp)))))))
 > +          (insert-file-contents fn)
 > +          (catch 'found
 > +            (let (sexp)
 > +              (while (setq sexp (let ((read-circle nil))
 > +	                          (condition-case nil
 > +		                      (read (current-buffer))
 > +		                    (end-of-file nil))))
 > +                (when (eq (car-safe sexp) 'deftheme)
 > +	          (throw 'found (setq doc (nth 2 sexp))))))))))
 >      (princ "\n\nDocumentation:\n")
 >      (princ (if (stringp doc)
 >  	       (substitute-command-keys doc)
 >
 > However for the modus themes it will fail as well because deftheme is
 > nested inside a eval-when-compile.

I feel like if there are more themes that suffer from this problem,
they could solve it by following the convention.  And for other themes,
it seems like either way we have to give it more thought, because AFAICS
following the convention is more difficult and the patch doesn't solve
it either.

 > And while I am at it, the docstring of Leuven-dark is wrong (guess it has
 > been copy/pasted from Leuven without modifications).

This has been fixed already, thanks.