GNU bug report logs - #57639
[PATCH] Add new command 'toggle-theme'

Previous Next

Package: emacs;

Reported by: Philip Kaludercic <philipk <at> posteo.net>

Date: Wed, 7 Sep 2022 07:20:01 UTC

Severity: wishlist

Tags: patch

Done: Philip Kaludercic <philipk <at> posteo.net>

Bug is archived. No further changes may be made.

Full log


Message #155 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 21 Sep 2022 13:38:13 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> Ok, that sounds good.  Here is the updated patch:

[...]

> This is actually done a few times by the modus themes and at least once
> by `leuven-dark' (see `leuven-dark-scale-font', tough I don't see why,
> and if the autoloads aren't being generated to begin with the cookie is
> pointless anyway).
> 
> Should this be addressed before the patch is pushed?

Hm...  are these themes also distributed via ELPA or something?  But in
any case, I don't see why you'd have:

;;;###autoload
(defun leuven-dark-scale-font (control default-height)

If you've activated the theme, you've loaded the file, so autoloading a
function like that doesn't seem helpful in any case.

So I think that sounds like it's just a mistake, and the ;;;###autoload
should be removed.  And the same with the commands autoloaded in modus
themes?  But perhaps there's a reason; I've added Prot to the CCs.
Perhaps he can clarify why those autoloads are in modus*.el.

But this looks more problematic:

;;;###autoload
(when (and (boundp 'custom-theme-load-path)
           load-file-name)
  ;; Add theme folder to `custom-theme-load-path' when installing over MELPA.
  (add-to-list 'custom-theme-load-path
               (file-name-as-directory (file-name-directory load-file-name))))

We don't want that in the Emacs loaddefs file, so just adding etc/themes
to our Makefile won't be the right thing to do, and we have to find a
different way to fix this.

Uhm...  Uhm...  I don't immediately see a good way to fix this...




This bug report was last modified 2 years and 215 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.