unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
@ 2023-08-23  8:44 Thierry Volpiatto
  2023-08-23  9:53 ` Mauro Aranda
  2023-08-23 11:47 ` Eli Zaretskii
  0 siblings, 2 replies; 13+ messages in thread
From: Thierry Volpiatto @ 2023-08-23  8:44 UTC (permalink / raw)
  To: 65468


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

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

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.

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


In GNU Emacs 29.1 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2023-08-16 built on IPad-S340
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Linux Mint 21.2

Configured using:
 'configure CFLAGS=-O8 --bindir=/usr/local/sbin/emacs-29.1
 --with-mailutils --with-cairo --with-x-toolkit=lucid
 --without-tree-sitter --without-native-compilation'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XAW3D XDBE XIM XINPUT2 XPM LUCID ZLIB

Important settings:
  value of $LANG: fr_FR.UTF-8
  locale-coding-system: utf-8-unix

Major mode: 

Minor modes in effect:
  emms-mode-line-mode: t
  emms-playing-time-display-mode: t
  emms-playing-time-mode: t
  bug-reference-prog-mode: t
  server-mode: t
  psession-mode: t
  psession-savehist-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  global-git-gutter-mode: t
  git-gutter-mode: t
  display-time-mode: t
  winner-mode: t
  tv-save-place-mode: t
  helm-epa-mode: t
  helm-descbinds-mode: t
  helm-top-poll-mode: t
  helm-adaptive-mode: t
  helm-mode: t
  helm-minibuffer-history-mode: t
  helm-ff-icon-mode: t
  shell-dirtrack-mode: t
  helm-popup-tip-mode: t
  async-bytecomp-package-mode: t
  dired-async-mode: t
  minibuffer-depth-indicate-mode: t
  gcmh-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/thierry/.emacs.d/elpa/boxquote-20220919.714/boxquote hides ~/elisp/boxquote

Features:
(shadow epa-mail face-remap emacsbug addressbook-bookmark tv-mu4e-config
config-w3m mu4e-contrib eshell esh-cmd generator esh-ext esh-opt
esh-proc esh-io esh-arg esh-module esh-groups esh-util mu4e-patch mu4e
mu4e-org org-config ob-gnuplot org-crypt org-protocol org ob ob-tangle
ob-ref ob-lob ob-table ob-exp org-macro org-src ob-comint org-pcomplete
org-list org-footnote org-faces org-entities ob-emacs-lisp ob-core
ob-eval org-cycle org-table ol org-fold org-fold-core org-keys oc
org-loaddefs org-version org-compat org-macs mu4e-notification
notifications mu4e-main mu4e-view mu4e-mime-parts gnus-art mm-uu mml2015
mm-view mml-smime smime gnutls dig gnus-sum gnus-group gnus-undo
gnus-start gnus-dbus gnus-cloud nnimap nnmail mail-source utf7 nnoo
gnus-spec gnus-int gnus-range gnus-win gnus nnheader range appt
diary-lib diary-loaddefs cal-menu calendar cal-loaddefs mu4e-headers
mu4e-thread mu4e-compose mu4e-draft mu4e-actions smtpmail mu4e-search
mu4e-lists mu4e-bookmarks mu4e-mark mu4e-message shr pixel-fill kinsoku
url-file svg dom flow-fill hl-line mu4e-contacts mu4e-update
mu4e-folders mu4e-context mu4e-query-items mu4e-server mu4e-modeline
mu4e-vars mu4e-helpers mu4e-config mu4e-window ido message sendmail
yank-media puny rfc822 mml mml-sec gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums
mail-prsvr mailabbrev mail-utils gmm-utils mailheader mu4e-obsolete
smerge-mode whitespace cl-indent helm-ring helm-x-files helm-for-files
helm-bookmark bookmark emms-config emms-librefm-stream
emms-librefm-scrobbler emms-playlist-limit emms-i18n emms-history
emms-score emms-stream-info emms-metaplaylist-mode emms-bookmarks
emms-cue emms-mode-line-icon emms-browser sort emms-volume
emms-volume-sndioctl emms-volume-mixerctl emms-volume-pulse
emms-volume-amixer emms-playlist-sort emms-last-played emms-player-xine
emms-player-mpd tq emms-lyrics emms-url emms-streams emms-show-all
emms-tag-editor emms-tag-tracktag emms-mark emms-mode-line emms-cache
emms-info-native bindat emms-info-exiftool emms-info-tinytag
emms-info-metaflac emms-info-opusinfo emms-info-ogginfo
emms-info-mp3info emms-playlist-mode emms-player-vlc emms-player-mpv
emms-playing-time emms-info emms-later-do emms-player-mplayer
emms-player-simple emms-source-playlist emms-source-file locate
emms-setup emms emms-compat emms-auto helm-external helm-net
modus-vivendi-theme modus-operandi-theme modus-themes cus-theme cl-extra
tramp-archive tramp-gvfs tramp-cache time-stamp zeroconf dbus xml
helm-command helm-elisp helm-eval edebug debug backtrace find-func
helm-info helm-ls-git vc-git diff-mode vc vc-dispatcher emacs-news-mode
noutline outline make-mode flymake-shellcheck cus-start flymake-proc
flymake project warnings thingatpt sh-script smie treesit executable
jka-compr bug-reference ef-winter-theme ef-tritanopia-dark-theme
ef-trio-dark-theme ef-symbiosis-theme ef-night-theme ef-maris-dark-theme
ef-elea-dark-theme ef-duo-dark-theme ef-deuteranopia-dark-theme
ef-dark-theme ef-cherie-theme ef-bio-theme ef-autumn-theme
ef-tritanopia-light-theme ef-trio-light-theme ef-summer-theme
ef-spring-theme ef-maris-light-theme ef-light-theme ef-kassio-theme
ef-frost-theme ef-elea-light-theme ef-duo-light-theme
ef-deuteranopia-light-theme ef-day-theme ef-cyprus-theme ef-themes
server imenu psession frameset undo-tree diff queue pcase git-gutter
mule-util dired-extension time winner describe-variable help-fns
radix-tree help-mode tv-utils tv-save-place.el advice init-helm epa
derived epg rfc6068 epg-config helm-epa isl helm-descbinds all-the-icons
all-the-icons-faces data-material data-weathericons data-octicons
data-fileicons data-faicons data-alltheicons cus-edit pp icons wid-edit
helm-sys popup helm-adaptive helm-mode helm-misc helm-files image-dired
image-dired-tags image-dired-external image-dired-util xdg image-mode
exif filenotify tramp tramp-loaddefs trampver tramp-integration files-x
tramp-compat rx shell pcomplete parse-time iso8601 time-date
helm-buffers helm-occur helm-tags helm-locate helm-grep wgrep-helm wgrep
grep compile text-property-search comint ansi-osc ring helm-regexp
format-spec ansi-color helm-utils helm-help helm-types
helm-extensions-autoloads helm-autoloads helm helm-global-bindings
helm-easymenu edmacro kmacro helm-core easy-mmode async-bytecomp
helm-source helm-multi-match helm-lib dired-async async dired-aux dired
dired-loaddefs mb-depth avoid cus-load gcmh boxquote-autoloads
ef-themes-autoloads gcmh-autoloads ledger-mode-autoloads
markdown-mode-autoloads osm-autoloads compat-autoloads info w3m-load
w3m-autoloads package browse-url url url-proxy url-privacy url-expand
url-methods url-history url-cookie generate-lisp-file url-domsuf
url-util mailcap url-handlers url-parse auth-source cl-seq eieio
eieio-core cl-macs password-cache json subr-x map byte-opt gv bytecomp
byte-compile url-vars cl-loaddefs cl-lib rmc iso-transl tooltip cconv
eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd
fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow
isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo x-toolkit
xinput2 x multi-tty make-network-process emacs)

Memory information:
((conses 16 1071049 1037182)
 (symbols 48 37733 180)
 (strings 32 199434 99964)
 (string-bytes 1 6124602)
 (vectors 16 77921)
 (vector-slots 8 1698366 1015829)
 (floats 8 1766 4594)
 (intervals 56 5971 3113)
 (buffers 976 117))
<#secure method=pgpmime mode=sign>

-- 
Thierry





^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-08-23  8:44 bug#65468: 29.1; describe-theme fails to describe some themes not loaded Thierry Volpiatto
@ 2023-08-23  9:53 ` Mauro Aranda
  2023-08-23 11:01   ` Thierry Volpiatto
  2023-08-23 11:57   ` Eli Zaretskii
  2023-08-23 11:47 ` Eli Zaretskii
  1 sibling, 2 replies; 13+ messages in thread
From: Mauro Aranda @ 2023-08-23  9:53 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: 65468

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.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-08-23  9:53 ` Mauro Aranda
@ 2023-08-23 11:01   ` Thierry Volpiatto
  2023-08-23 11:57   ` Eli Zaretskii
  1 sibling, 0 replies; 13+ messages in thread
From: Thierry Volpiatto @ 2023-08-23 11:01 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: 65468

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


Mauro Aranda <maurooaranda@gmail.com> writes:

> 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.

Maybe.

>>
>> 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.

Yes.

>> 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.

For the themes that are in Emacs, should be easy to fix.

> 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.

It seems the themes installed from package are already loaded, not sure
if it is true for all of them though.

For what is `describe-theme-1` patch is is always better to cover more
use cases even if not all cases are covered (e.g. modus themes), however
having a new function that fetch documentation from file would be great
and reusable elsewhere (I have here - in helm - to write my own function to get the
first line documentation of a theme).

>
>> 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.

Good, thanks.

-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-08-23  8:44 bug#65468: 29.1; describe-theme fails to describe some themes not loaded Thierry Volpiatto
  2023-08-23  9:53 ` Mauro Aranda
@ 2023-08-23 11:47 ` Eli Zaretskii
  2023-08-23 13:12   ` Thierry Volpiatto
  1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2023-08-23 11:47 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: 65468

> From: Thierry Volpiatto <thievol@posteo.net>
> Date: Wed, 23 Aug 2023 08:44:16 +0000
> 
> 
> This because `describe-theme-1` is not looping in buffer to find
> `deftheme` definition.

We never did better, did we?  IOW, this issue exists for a long time,
right?

If so, I think this should go to master, not to the emacs-29 branch.

Thanks.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-08-23  9:53 ` Mauro Aranda
  2023-08-23 11:01   ` Thierry Volpiatto
@ 2023-08-23 11:57   ` Eli Zaretskii
  2023-08-24 10:16     ` Mauro Aranda
  1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2023-08-23 11:57 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: thievol, 65468

> Cc: 65468@debbugs.gnu.org
> Date: Wed, 23 Aug 2023 06:53:03 -0300
> From: 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.

I think we should indeed fix the themes that come with Emacs.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-08-23 11:47 ` Eli Zaretskii
@ 2023-08-23 13:12   ` Thierry Volpiatto
  2023-08-26  7:57     ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Thierry Volpiatto @ 2023-08-23 13:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 65468

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


Eli Zaretskii <eliz@gnu.org> writes:

>> From: Thierry Volpiatto <thievol@posteo.net>
>> Date: Wed, 23 Aug 2023 08:44:16 +0000
>> 
>> 
>> This because `describe-theme-1` is not looping in buffer to find
>> `deftheme` definition.
>
> We never did better, did we?  IOW, this issue exists for a long time,
> right?
>
> If so, I think this should go to master, not to the emacs-29 branch.

I will not push anything, just proposing patch if any interest to fix
this issue, here a new patch that provide a separate function for this
part of code:

diff --git a/lisp/cus-theme.el b/lisp/cus-theme.el
index 5d3f2585976..c6c9d9c892b 100644
--- a/lisp/cus-theme.el
+++ b/lisp/cus-theme.el
@@ -490,6 +490,29 @@ It includes all faces in list FACES."
     (with-current-buffer standard-output
       (describe-theme-1 theme))))
 
+(defun describe-theme-from-file (&optional file short)
+  "Describe theme from its file FILE without loading it.
+
+If FILE is nil try to find the file from the theme name in
+`custom-theme-load-path'.
+If SHORT is non nil show only the first line of documentation."
+    (let ((file (or file
+                  (locate-file (concat (symbol-name theme) "-theme.el")
+			       (custom-theme--load-path)
+			       '("" "c")))))
+      (with-temp-buffer
+        (insert-file-contents file)
+        (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 (if short
+                                  (car (split-string (nth 2 sexp) "\n"))
+                                (nth 2 sexp))))))))))
+
 (defun describe-theme-1 (theme)
   (prin1 theme)
   (princ " is a custom theme")
@@ -510,16 +533,9 @@ It includes all faces in list FACES."
 	    (princ "It is loaded but disabled."))
 	  (setq doc (get theme 'theme-documentation)))
       (princ "It is not loaded.")
-      ;; Attempt to grab the theme documentation
+      ;; Attempt to grab the theme documentation from file.
       (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)))))))
+	(setq doc (describe-theme-from-file fn))))
     (princ "\n\nDocumentation:\n")
     (princ (if (stringp doc)
 	       (substitute-command-keys doc)

-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-08-23 11:57   ` Eli Zaretskii
@ 2023-08-24 10:16     ` Mauro Aranda
  2023-08-25  3:48       ` Protesilaos Stavrou
  0 siblings, 1 reply; 13+ messages in thread
From: Mauro Aranda @ 2023-08-24 10:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: thievol, 65468, Protesilaos Stavrou

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

Eli Zaretskii <eliz@gnu.org> writes:

 >> Cc: 65468@debbugs.gnu.org
 >> Date: Wed, 23 Aug 2023 06:53:03 -0300
 >> From: 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.
 >
 > I think we should indeed fix the themes that come with Emacs.

Here's a patch for the leuven themes.  And I'm CCing Prot so he can take
a look to adjust the modus-themes.

[-- Attachment #2: 0001-Adjust-built-in-themes-to-convention-Bug-65468.patch --]
[-- Type: text/x-patch, Size: 2505 bytes --]

From ab417284f27d172cc95f4c735f28bdfd6366817b Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Thu, 24 Aug 2023 07:12:41 -0300
Subject: [PATCH] Adjust built-in themes to convention (Bug#65468)

* etc/themes/leuven-dark-theme.el
* etc/themes/leuven-theme.el: Make the deftheme call the first form.
---
 etc/themes/leuven-dark-theme.el | 20 ++++++++++----------
 etc/themes/leuven-theme.el      | 20 ++++++++++----------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/etc/themes/leuven-dark-theme.el b/etc/themes/leuven-dark-theme.el
index 33a15945e71..a2445896ea6 100644
--- a/etc/themes/leuven-dark-theme.el
+++ b/etc/themes/leuven-dark-theme.el
@@ -39,6 +39,16 @@
 
 ;;; Code:
 
+;;;###theme-autoload
+(deftheme leuven-dark
+  "Face colors with a dark background.
+Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
+Flyspell, Semantic, and Ansi-Color faces are included -- and much
+more..."
+  :background-mode 'dark
+  :family 'leuven
+  :kind 'color-scheme)
+
 ;;; Options.
 
 (defgroup leuven-dark nil
@@ -93,16 +103,6 @@ leuven-dark-scale-font
 
 ;;; Theme Faces.
 
-;;;###theme-autoload
-(deftheme leuven-dark
-  "Face colors with a dark background.
-Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
-Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more..."
-  :background-mode 'dark
-  :family 'leuven
-  :kind 'color-scheme)
-
 (let ((class '((class color) (min-colors 89)))
 
       ;; Leuven generic colors.
diff --git a/etc/themes/leuven-theme.el b/etc/themes/leuven-theme.el
index f7d454381d7..3d8d0d49b36 100644
--- a/etc/themes/leuven-theme.el
+++ b/etc/themes/leuven-theme.el
@@ -38,6 +38,16 @@
 
 ;;; Code:
 
+;;;###theme-autoload
+(deftheme leuven
+  "Face colors with a light background.
+Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
+Flyspell, Semantic, and Ansi-Color faces are included -- and much
+more..."
+  :background-mode 'light
+  :kind 'color-scheme
+  :family 'leuven)
+
 ;;; Options.
 
 (defgroup leuven nil
@@ -74,16 +84,6 @@ leuven-scale-font
 
 ;;; Theme Faces.
 
-;;;###theme-autoload
-(deftheme leuven
-  "Face colors with a light background.
-Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
-Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more..."
-  :background-mode 'light
-  :kind 'color-scheme
-  :family 'leuven)
-
 (let ((class '((class color) (min-colors 89)))
 
       ;; Leuven generic colors.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-08-24 10:16     ` Mauro Aranda
@ 2023-08-25  3:48       ` Protesilaos Stavrou
  0 siblings, 0 replies; 13+ messages in thread
From: Protesilaos Stavrou @ 2023-08-25  3:48 UTC (permalink / raw)
  To: Mauro Aranda, Eli Zaretskii; +Cc: thievol, 65468

> From: Mauro Aranda <maurooaranda@gmail.com>
> Date: Thu, 24 Aug 2023 07:16:10 -0300

> [... 26 lines elided]

> Here's a patch for the leuven themes.  And I'm CCing Prot so he can take
> a look to adjust the modus-themes.

> [... 88 lines elided]

Thank you Mauro!  I just made the changes on my end, though not directly
on emacs.git.  I will prepare a new version of the themes as soon as
possible and sync that.  Hopefully this week.

-- 
Protesilaos Stavrou
https://protesilaos.com





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-08-23 13:12   ` Thierry Volpiatto
@ 2023-08-26  7:57     ` Eli Zaretskii
  2023-08-26 15:31       ` Thierry Volpiatto
  2023-09-09 11:33       ` Mauro Aranda
  0 siblings, 2 replies; 13+ messages in thread
From: Eli Zaretskii @ 2023-08-26  7:57 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: 65468

> From: Thierry Volpiatto <thievol@posteo.net>
> Cc: 65468@debbugs.gnu.org
> Date: Wed, 23 Aug 2023 13:12:28 +0000
> 
> > If so, I think this should go to master, not to the emacs-29 branch.
> 
> I will not push anything, just proposing patch if any interest to fix
> this issue, here a new patch that provide a separate function for this
> part of code:

Thanks, installed on master (after fixing a small blunder).





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-08-26  7:57     ` Eli Zaretskii
@ 2023-08-26 15:31       ` Thierry Volpiatto
  2023-09-09 11:33       ` Mauro Aranda
  1 sibling, 0 replies; 13+ messages in thread
From: Thierry Volpiatto @ 2023-08-26 15:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 65468

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


Eli Zaretskii <eliz@gnu.org> writes:

>> From: Thierry Volpiatto <thievol@posteo.net>
>> Cc: 65468@debbugs.gnu.org
>> Date: Wed, 23 Aug 2023 13:12:28 +0000
>> 
>> > If so, I think this should go to master, not to the emacs-29 branch.
>> 
>> I will not push anything, just proposing patch if any interest to fix
>> this issue, here a new patch that provide a separate function for this
>> part of code:
>
> Thanks, installed on master (after fixing a small blunder).

I see, thanks.

-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-08-26  7:57     ` Eli Zaretskii
  2023-08-26 15:31       ` Thierry Volpiatto
@ 2023-09-09 11:33       ` Mauro Aranda
  2023-09-09 11:47         ` Eli Zaretskii
  2023-09-09 11:51         ` Stefan Kangas
  1 sibling, 2 replies; 13+ messages in thread
From: Mauro Aranda @ 2023-09-09 11:33 UTC (permalink / raw)
  To: Eli Zaretskii, Thierry Volpiatto; +Cc: 65468

On 26/8/23 04:57, Eli Zaretskii wrote:
>> From: Thierry Volpiatto <thievol@posteo.net>
>> Cc: 65468@debbugs.gnu.org
>> Date: Wed, 23 Aug 2023 13:12:28 +0000
>>
>>> If so, I think this should go to master, not to the emacs-29 branch.
>>
>> I will not push anything, just proposing patch if any interest to fix
>> this issue, here a new patch that provide a separate function for this
>> part of code:
> 
> Thanks, installed on master (after fixing a small blunder).
> 

It seems to me that there's nothing left to do here.  The patch by
Thierry got installed, I fixed the leuven themes and Prot reported that
he fixed the modus-themes on his end.

Can we close it? Or did I miss something?






^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-09-09 11:33       ` Mauro Aranda
@ 2023-09-09 11:47         ` Eli Zaretskii
  2023-09-09 11:51         ` Stefan Kangas
  1 sibling, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2023-09-09 11:47 UTC (permalink / raw)
  To: Mauro Aranda, Stefan Kangas; +Cc: thievol, 65468

> Date: Sat, 9 Sep 2023 08:33:27 -0300
> Cc: 65468@debbugs.gnu.org
> From: Mauro Aranda <maurooaranda@gmail.com>
> 
> On 26/8/23 04:57, Eli Zaretskii wrote:
> >> From: Thierry Volpiatto <thievol@posteo.net>
> >> Cc: 65468@debbugs.gnu.org
> >> Date: Wed, 23 Aug 2023 13:12:28 +0000
> >>
> >>> If so, I think this should go to master, not to the emacs-29 branch.
> >>
> >> I will not push anything, just proposing patch if any interest to fix
> >> this issue, here a new patch that provide a separate function for this
> >> part of code:
> > 
> > Thanks, installed on master (after fixing a small blunder).
> > 
> 
> It seems to me that there's nothing left to do here.  The patch by
> Thierry got installed, I fixed the leuven themes and Prot reported that
> he fixed the modus-themes on his end.
> 
> Can we close it? Or did I miss something?

Fine by me.  Stefan?





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#65468: 29.1; describe-theme fails to describe some themes not loaded
  2023-09-09 11:33       ` Mauro Aranda
  2023-09-09 11:47         ` Eli Zaretskii
@ 2023-09-09 11:51         ` Stefan Kangas
  1 sibling, 0 replies; 13+ messages in thread
From: Stefan Kangas @ 2023-09-09 11:51 UTC (permalink / raw)
  To: Mauro Aranda, Eli Zaretskii, Thierry Volpiatto; +Cc: 65468-done

Mauro Aranda <maurooaranda@gmail.com> writes:

> It seems to me that there's nothing left to do here.  The patch by
> Thierry got installed, I fixed the leuven themes and Prot reported that
> he fixed the modus-themes on his end.

OK, let's close it.





^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-09-09 11:51 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-23  8:44 bug#65468: 29.1; describe-theme fails to describe some themes not loaded Thierry Volpiatto
2023-08-23  9:53 ` Mauro Aranda
2023-08-23 11:01   ` Thierry Volpiatto
2023-08-23 11:57   ` Eli Zaretskii
2023-08-24 10:16     ` Mauro Aranda
2023-08-25  3:48       ` Protesilaos Stavrou
2023-08-23 11:47 ` Eli Zaretskii
2023-08-23 13:12   ` Thierry Volpiatto
2023-08-26  7:57     ` Eli Zaretskii
2023-08-26 15:31       ` Thierry Volpiatto
2023-09-09 11:33       ` Mauro Aranda
2023-09-09 11:47         ` Eli Zaretskii
2023-09-09 11:51         ` Stefan Kangas

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).