Eli Zaretskii writes: >> From: Philip Kaludercic >> Cc: larsi@gnus.org, 57639@debbugs.gnu.org >> Date: Sat, 17 Sep 2022 21:33:18 +0000 >> >> >> + (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? >> >> Strictly speaking that error message would be wrong at this point, >> because we cannot say if a theme has no variants if it is not part of a >> family. This is because variants of a theme are all those that are part >> of the same family. I think it would be better to clarify this in the >> documentation. > > But the documentation doesn't explain what it means for a theme to be > part of a family. Specifically, how does one tell, by looking at a > theme, whether it is or isn't part of a family? > > An alternative for what I suggested above is to say > > Theme `%s' does not have any known variants I get what you mean... how about (error "Theme `%s' is not part of a family of variants" theme) ? >> +(defun theme-choose-variant (&optional no-confirm no-enable) >> + "Prompt to switch from the current theme to a variant. > ^^^^^^^^^^^^ > "to one of its variants" is more clear. Done. >> +Themes only have variants if they are part of a family of themes. > > This should explain what it means to be part of a family, otherwise > this sentence is not helpful. My intention was for this to be an explanation. The issue is that variants and family are mutually recursive concepts: - A variant of a theme are those which are part of the same family - A family of themes is the set of all variants of a theme. Perhaps it is just easier to collapse both concepts into either variant of family and just "expose" by documenting it. I've tried doing so below. >> +The current theme will be immediately disabled before variant is >> +enabled. > > The "immediately" part should probably be removed, as it doesn't seem > to convey anything of importance, does it? Come to think of it, what > exactly does this sentence try to say? isn't it obvious that the > current theme will be disabled and then the variant will be enabled? I guess it isn't necessary, the point was just to emphasise that nothing happens between disabling the previous theme and enabling the variant. There was something I had in mind when writing this initially, but I cannot recall it anymore. So I've removed it for now. >> In case the current theme has only one variant, it will >> +be toggled without prompting. > > "toggled" is wrong here. I suggest > > If the current theme has only one variant, switch to that variant > without prompting, otherwise prompt for the variant to select. Done. >> For NO-CONFIRM and NO-ENABLE, see >> +`load-theme'." > > I suggest to say this the other way around: > > See `load-theme' for the meaning of NO-CONFIRM and NO-ENABLE. Done. > Thanks.