From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#57639: [PATCH] Add new command 'toggle-theme' Date: Sat, 17 Sep 2022 21:32:42 +0300 Message-ID: <831qs926np.fsf@gnu.org> References: <875yhzmz25.fsf@posteo.net> <875yhzmj50.fsf@gnus.org> <875yhzl3a1.fsf@posteo.net> <87fsh25bso.fsf@gnus.org> <871qsil3jm.fsf@posteo.net> <87fsgygo7c.fsf@gnus.org> <87a676jfx2.fsf@posteo.net> <87bkrmdt46.fsf@gnus.org> <8735cxpx2r.fsf@posteo.net> <87k066yyn0.fsf@posteo.net> <877d269kgv.fsf@gnus.org> <87bkriyqci.fsf@posteo.net> <874jxa6kyj.fsf@gnus.org> <87o7vdg97w.fsf@posteo.net> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="26005"; mail-complaints-to="usenet@ciao.gmane.io" Cc: larsi@gnus.org, 57639@debbugs.gnu.org To: Philip Kaludercic Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Sep 17 20:33:34 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oZcdB-0006d0-Jq for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 17 Sep 2022 20:33:33 +0200 Original-Received: from localhost ([::1]:41658 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oZcdA-00068D-Ic for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 17 Sep 2022 14:33:32 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:53824) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oZccg-0005uF-Qk for bug-gnu-emacs@gnu.org; Sat, 17 Sep 2022 14:33:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:48115) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oZccg-0003Fk-IL for bug-gnu-emacs@gnu.org; Sat, 17 Sep 2022 14:33:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oZccg-0000h5-Di for bug-gnu-emacs@gnu.org; Sat, 17 Sep 2022 14:33:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 17 Sep 2022 18:33:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 57639 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 57639-submit@debbugs.gnu.org id=B57639.16634395722650 (code B ref 57639); Sat, 17 Sep 2022 18:33:02 +0000 Original-Received: (at 57639) by debbugs.gnu.org; 17 Sep 2022 18:32:52 +0000 Original-Received: from localhost ([127.0.0.1]:47193 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oZccW-0000gf-2y for submit@debbugs.gnu.org; Sat, 17 Sep 2022 14:32:52 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:44882) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oZccS-0000gO-Ie for 57639@debbugs.gnu.org; Sat, 17 Sep 2022 14:32:50 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:44120) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oZccN-0003Es-8f; Sat, 17 Sep 2022 14:32:43 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=mjjvJyJId7bXtVV1KUroY6QLzIrwT/p+tef1IY/ckX8=; b=kIXt5cGpV8LC GXPsmXjRZ8ZR/c+cbTx5PuNUMm1jY2rajQMo7GZ3SaTW4FOIJesJhls02UPgl4zavHTmLeOMOuOSe 2iGbiIjeIT7Lp98XlQc1UFT63RzTGvQ/p5mQQBeFv2lqdIP8NJM1MekO3qirTqUjB+8N1OAbP3e3N YsK2WMYEnEwSt9yquW6aL2srWyBaXaU+eYEdmSk2aFJyH/bkG18kn5M1NfapdgYLqizaRVXKSOJ2L dzfK4bC+ABlciU9B25ENfl24lvpdTG2JRpTiqZl7ojgu7GSRAOQruMF5YEdLnlR7WmQkn0p+0rVoN Qq+8h3xIrzZ07mi/di9fDQ==; Original-Received: from [87.69.77.57] (port=2234 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oZccM-0000dt-Nb; Sat, 17 Sep 2022 14:32:43 -0400 In-Reply-To: <87o7vdg97w.fsf@posteo.net> (message from Philip Kaludercic on Sat, 17 Sep 2022 18:13:39 +0000) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:242905 Archived-At: > Cc: 57639@debbugs.gnu.org > From: Philip Kaludercic > Date: Sat, 17 Sep 2022 18:13:39 +0000 > > +@findex theme-choose-variant > + Some themes have variants (most often these are light and dark > +pairs). You can switch between these by typing @kbd{M-x > +theme-choose-variant}. Note that this only works if only one theme is > +active. If a theme has only one alternative, it will toggle > +automatically. If there are more of them, it will query which one to > +use. This description is confusing, I needed to read it several times before I understood what you were trying to say. The main problem is that the "Note" sentence doesn't belong, and it interrupts the logic of the description. Here's my suggestion for saying it more clearly: Some themes have variants (most often just two: light and dark). You can switch to another variant with @kbd{M-x theme-choose-variant}. If the currently active theme has only one other variant, it will be selected; if there are more variants, the command will prompt you which one to switch to. Note that @code{theme-choose-variant} only works if a single theme is active. (Btw, what happens if more than one theme is active and the user invokes theme-choose-variant? should this be described?) > +Themes*} buffer. The remaining arguments @var{properties} is used > +pass a property list with theme attributes. ^^^^^^^ "are used", in plural. > + :brightness 'light Should we use :background-mode instead of :brightness, for consistency with frame-background-mode? > +(defun theme-choose-variant (&optional no-confirm no-enable) > + "Toggle the current active theme by enabling its dual pair. "Toggle ... by enabling"? "Dual pair"? Can this sentence be rephrased to be more clear? > +The current theme will be immediately disabled before the dual > +theme has been enabled. Likewise here: "dual theme" doesn't explain itself, and seems to be inaccurate, given the description in the manual. > If THEME is not active an error will be > +raised. Passive tense alert! > If theme is nil For NO-CONFIRM and NO-ENABLE, see > +`load-theme'." Something's missing here or wrong with the punctuation? > + ((length> custom-enabled-themes 1) > + (user-error "More than one theme active, cannot unambiguously toggle"))) Wouldn't it be better to prompt for the theme in this case? > + (let* ((theme (car custom-enabled-themes)) > + (family (plist-get (get theme 'theme-properties) :family))) > + (unless family > + (error "`%s' is not part of a family" theme)) "Family"? this terminology was never mentioned in the manual or the doc string. How about Theme `%s' does not have any variants instead? Thanks.