~protesilaos/modus-themes

3 2

Support extend in modus-themes-completions

Details
Message ID
<e8b0e604-5a12-d06d-f4db-06874fd755e0@inventati.org>
DKIM signature
pass
Download raw message
Hi Prot,

I have this code in my init.el:

(defun mu-load-modus-operandi ()
   "Load `modus-operandi' theme."
   (load-theme 'modus-operandi t)
   (with-eval-after-load 'vertico
     (set-face-attribute 'vertico-current nil :extend nil)))
(add-hook 'window-setup-hook #'mu-load-modus-operandi)

As you can see it just disables `:extend` for `vertico-current' after 
loading `modus-operandi.

The thing is I do not use Marginalia and I see no point for the 
highlight of the current Vertico match to extend across all the screen.

Do you think something like 'no-extend` can be added to 
`modus-themes-completions`?

-- 
Manuel Uberti
https://manueluberti.eu
Details
Message ID
<87wnfiekno.fsf@protesilaos.com>
In-Reply-To
<e8b0e604-5a12-d06d-f4db-06874fd755e0@inventati.org> (view parent)
DKIM signature
pass
Download raw message
Patch: +11 -2
> From: Manuel Uberti <manuel.uberti@inventati.org>
> Date: Thu, 21 Apr 2022 12:58:20 +0200
>
> Hi Prot,

Hello Manuel!

> I have this code in my init.el:
>
> (defun mu-load-modus-operandi ()
>    "Load `modus-operandi' theme."
>    (load-theme 'modus-operandi t)
>    (with-eval-after-load 'vertico
>      (set-face-attribute 'vertico-current nil :extend nil)))
> (add-hook 'window-setup-hook #'mu-load-modus-operandi)
>
> As you can see it just disables `:extend` for `vertico-current' after 
> loading `modus-operandi.
>
> The thing is I do not use Marginalia and I see no point for the 
> highlight of the current Vertico match to extend across all the screen.
>
> Do you think something like 'no-extend` can be added to 
> `modus-themes-completions`?

I tried with the diff that I copy further below.  It does not work as
expected due to the peculiar nature of ':extend'.  Unlike other face
attributes, if the theme does not explicitly override its value, it
retains it from its original face definition.  In other words, the
':extend t' for the 'vertico-current' face comes directly from
vertico.el, not the modus-themes.

If we use an "optionally do not extend" logic, we have to explicitly
override the ':extend t' with ':extend unspecified'.  That works.
Though it means that the new default is now set by the themes, instead
of the underlying package.  In this case, the themes would disable the
extended background.

By the same token, if we use the "optionally extend" logic we again
disable the effect by default.  And we also make a promise we cannot
keep, as some UIs do not extend the background regardless of what we do
(because there needs to be a newline at the end).

Maybe I am missing something obvious now.  Otherwise I cannot think how
to do this without affecting the default looks.

What do you think?



 modus-themes.el | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/modus-themes.el b/modus-themes.el
index d5ed7ad..76460b4 100644
--- a/modus-themes.el
+++ b/modus-themes.el
@@ -2541,6 +2541,11 @@ (defcustom modus-themes-completions nil

- `italic' to use a slanted font (italic or oblique forms);

- `no-extend' to disable the extension of the background to the
  edge of the window (otherwise we leave it up to the given
  package to make this decision---e.g. Vertico extends by
  default);

- The symbol of a font weight attribute such as `light',
  `semibold', et cetera.  Valid symbols are defined in the
  variable `modus-themes-weights'.  The absence of a weight means
@@ -2583,7 +2588,7 @@ (defcustom modus-themes-completions nil
Also refer to the Orderless documentation for its intersection
with Company (if you choose to use those in tandem)."
  :group 'modus-themes
  :package-version '(modus-themes . "2.3.0")
  :package-version '(modus-themes . "2.4.0")
  :version "29.1"
  :type `(set
          (cons :tag "Matches"
@@ -2624,6 +2629,7 @@ (defcustom modus-themes-completions nil
                     (const :tag "With accented background" accented)
                     (const :tag "Increased coloration" intense)
                     (const :tag "Italic font (oblique or slanted forms)" italic)
                     (const :tag "Do not extend background (when applicable)" no-extend)
                     (const :tag "Underline" underline)))
          (cons :tag "Popup"
                (const popup)
@@ -2644,6 +2650,7 @@ (defcustom modus-themes-completions nil
                     (const :tag "With accented background" accented)
                     (const :tag "Increased coloration" intense)
                     (const :tag "Italic font (oblique or slanted forms)" italic)
                     (const :tag "Do not extend background (when applicable)" no-extend)
                     (const :tag "Underline" underline))))
  :set #'modus-themes--set-option
  :initialize #'custom-initialize-default
@@ -3896,6 +3903,8 @@ (defun modus-themes--completion-line (key bg fg bgintense fgintense &optional bg
      ('unspecified))
     :underline
     (if (memq 'underline properties) t 'unspecified)
     :extend
     (if (memq 'no-extend properties) nil 'unspecified)
     :weight
     (if (and weight (null bold)) weight 'unspecified))))

@@ -7522,7 +7531,7 @@ ;;;;; vc (vc-dir.el, vc-hooks.el)
    `(vc-state-base ((,class :foreground ,fg-active)))
    `(vc-up-to-date-state ((,class :foreground ,fg-special-cold)))
;;;;; vertico
    `(vertico-current ((,class :inherit modus-themes-completion-selected)))
    `(vertico-current ((,class :inherit modus-themes-completion-selected :extend unspecified)))
;;;;; vertico-quick
    `(vertico-quick1 ((,class :inherit bold :background ,bg-char-0)))
    `(vertico-quick2 ((,class :inherit bold :background ,bg-char-1)))




-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<40cf7845-36cc-855b-0bc5-3e94afdf1a64@inventati.org>
In-Reply-To
<87wnfiekno.fsf@protesilaos.com> (view parent)
DKIM signature
pass
Download raw message
On 21/04/22 13:32, Protesilaos Stavrou wrote:
> I tried with the diff that I copy further below.  It does not work as
> expected due to the peculiar nature of ':extend'.  Unlike other face
> attributes, if the theme does not explicitly override its value, it
> retains it from its original face definition.  In other words, the
> ':extend t' for the 'vertico-current' face comes directly from
> vertico.el, not the modus-themes.
> 
> If we use an "optionally do not extend" logic, we have to explicitly
> override the ':extend t' with ':extend unspecified'.  That works.
> Though it means that the new default is now set by the themes, instead
> of the underlying package.  In this case, the themes would disable the
> extended background.
> 
> By the same token, if we use the "optionally extend" logic we again
> disable the effect by default.  And we also make a promise we cannot
> keep, as some UIs do not extend the background regardless of what we do
> (because there needs to be a newline at the end).
> 
> Maybe I am missing something obvious now.  Otherwise I cannot think how
> to do this without affecting the default looks.
> 
> What do you think?
Since it is related to a setting in Vertico, I'd say don't change 
anything in the Modus Themes. I agree with you: it does not make much 
sense to alter an option which forces you to keep track of upstream 
changes or act differently according to different UIs.

I can still tweak it locally as I've been doing for a while, so 
everything's good.

-- 
Manuel Uberti
https://manueluberti.eu
Details
Message ID
<87tuamnu6l.fsf@protesilaos.com>
In-Reply-To
<40cf7845-36cc-855b-0bc5-3e94afdf1a64@inventati.org> (view parent)
DKIM signature
pass
Download raw message
> From: Manuel Uberti <manuel.uberti@inventati.org>
> Date: Thu, 21 Apr 2022 14:44:14 +0200
>
> On 21/04/22 13:32, Protesilaos Stavrou wrote:
>> I tried with the diff that I copy further below.  It does not work as
>> expected due to the peculiar nature of ':extend'.  Unlike other face
>> attributes, if the theme does not explicitly override its value, it
>> retains it from its original face definition.  In other words, the
>> ':extend t' for the 'vertico-current' face comes directly from
>> vertico.el, not the modus-themes.
>> 
>> If we use an "optionally do not extend" logic, we have to explicitly
>> override the ':extend t' with ':extend unspecified'.  That works.
>> Though it means that the new default is now set by the themes, instead
>> of the underlying package.  In this case, the themes would disable the
>> extended background.
>> 
>> By the same token, if we use the "optionally extend" logic we again
>> disable the effect by default.  And we also make a promise we cannot
>> keep, as some UIs do not extend the background regardless of what we do
>> (because there needs to be a newline at the end).
>> 
>> Maybe I am missing something obvious now.  Otherwise I cannot think how
>> to do this without affecting the default looks.
>> 
>> What do you think?
> Since it is related to a setting in Vertico, I'd say don't change 
> anything in the Modus Themes. I agree with you: it does not make much 
> sense to alter an option which forces you to keep track of upstream 
> changes or act differently according to different UIs.
>
> I can still tweak it locally as I've been doing for a while, so 
> everything's good.

Okay then.  Thank you!

-- 
Protesilaos Stavrou
https://protesilaos.com
Reply to thread Export thread (mbox)