unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* ELPA: New package: nano-theme
@ 2021-09-28 13:11 Nicolas P. Rougier (inria)
  2021-09-28 17:47 ` Philip Kaludercic
  0 siblings, 1 reply; 16+ messages in thread
From: Nicolas P. Rougier (inria) @ 2021-09-28 13:11 UTC (permalink / raw)
  To: emacs-devel


Dear all,

I would like to submit a new package to ELPA which is a theme 
based on Material colors (https://material.io/) for the light 
version and Nord colors (https://www.nordtheme.com/) for the dark 
version. The theme itself is made of (only) 6 different faces and 
the rationale is here: https://arxiv.org/abs/2008.06030

The theme is hosted at https://github.com/rougier/nano-theme and 
the README displays what it looks like. Note that the theme 
exploits face inheritance a lot and Eli explained me that it might 
degrade performances. I don't know if this needs to be fixed.

Best,
Nicolas



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

* Re: ELPA: New package: nano-theme
  2021-09-28 13:11 ELPA: New package: nano-theme Nicolas P. Rougier (inria)
@ 2021-09-28 17:47 ` Philip Kaludercic
  2021-09-28 17:57   ` Nicolas P. Rougier (inria)
  2021-10-01  9:59   ` Philip Kaludercic
  0 siblings, 2 replies; 16+ messages in thread
From: Philip Kaludercic @ 2021-09-28 17:47 UTC (permalink / raw)
  To: Nicolas P. Rougier (inria); +Cc: emacs-devel

"Nicolas P. Rougier (inria)" <nicolas.rougier@inria.fr> writes:

> Dear all,
>
> I would like to submit a new package to ELPA which is a theme based on
> Material colors (https://material.io/) for the light version and Nord
> colors (https://www.nordtheme.com/) for the dark version. The theme
> itself is made of (only) 6 different faces and the rationale is here:
> https://arxiv.org/abs/2008.06030
>
> The theme is hosted at https://github.com/rougier/nano-theme and the
> README displays what it looks like. Note that the theme exploits face
> inheritance a lot and Eli explained me that it might degrade
> performances. I don't know if this needs to be fixed.

I see that a lot of faces inherit from nano-default, nano-strong,
nano-faded, etc. Wouldn't it make more sense to inherit from built-in
faces and then let default, bold, shadow, etc. inherit from these faces.

Furthermore, I wonder why you define the commands nano-light and
nano-dark, instead of two themes, nano-light and nano-dark (along the
same lines of what modus-themes currently does). You could also turn
nano-setup into a custom theme, so that the user can easily enable and
disable it.

A few more points:

- Should the font really be part of the face? I think it makes sense to
  recommend a few fonts, but it is unconventional to add it as part of a
  theme.
- When you are only using one branch of a if expression, prefer
  when. Instead of (if (not ...) ...), prefer unless.
- Lines 363ff. seem to be indented unconventionally. Maybe add a
  .dir-locals.el to make sure everyone is using the same whitespace
  configuration.

Also, as you depend on at least 27.1, you probably don't have to check
if custom-theme-load-path is defined. The same applies to tooltip-mode,
scroll-bar-mode and tool-bar-mode. What you are doing looks more like
configuration code, where you want to make sure that a .emacs works on
older versions of Emacs.

> Best,
> Nicolas
>
>

-- 
	Philip Kaludercic



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

* Re: ELPA: New package: nano-theme
  2021-09-28 17:47 ` Philip Kaludercic
@ 2021-09-28 17:57   ` Nicolas P. Rougier (inria)
  2021-09-28 18:32     ` Stefan Kangas
  2021-09-28 18:37     ` Philip Kaludercic
  2021-10-01  9:59   ` Philip Kaludercic
  1 sibling, 2 replies; 16+ messages in thread
From: Nicolas P. Rougier (inria) @ 2021-09-28 17:57 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel


Many thanks for the review and the feedback.

Philip Kaludercic <philipk@posteo.net> writes:

> "Nicolas P. Rougier (inria)" <nicolas.rougier@inria.fr> writes:

> I see that a lot of faces inherit from nano-default, 
> nano-strong,
> nano-faded, etc. Wouldn't it make more sense to inherit from 
> built-in
> faces and then let default, bold, shadow, etc. inherit from 
> these faces.

Having such names helps a lot when you assign faces. For example, 
when I define font-lock-comment-face, I want to make it faded and 
it feels more natural and explicit to inherit from 
nano-faded. Also, is there an associated computational cost when 
defining a new face?

> Furthermore, I wonder why you define the commands nano-light and
> nano-dark, instead of two themes, nano-light and nano-dark 
> (along the
> same lines of what modus-themes currently does). You could also 
> turn
> nano-setup into a custom theme, so that the user can easily 
> enable and
> disable it.

Maybe I need to read the documentation on what a theme can 
set. For example, I set the face for the minibuffer (0 & 1) and 
echo area (0 & 1) and I wasn't sure how to specify this in a 
theme. Same for underline to be set at descent line, etc.

> A few more points:
>
> - Should the font really be part of the face? I think it makes 
> sense to
>   recommend a few fonts, but it is unconventional to add it as 
>   part of a
>   theme.

I agree this might be unconventional but since you can specify a 
font family for each face, why not use it? And user can still 
choose its own font stack.

> - When you are only using one branch of a if expression, prefer
>   when. Instead of (if (not ...) ...), prefer unless.

Thanks, I'll do that.


> - Lines 363ff. seem to be indented unconventionally. Maybe add a
>   .dir-locals.el to make sure everyone is using the same 
>   whitespace
>   configuration.

Not sure what you mean by add a .dir-locals.el.
Can you give me a pointer to the documentation?

> Also, as you depend on at least 27.1, you probably don't have to 
> check
> if custom-theme-load-path is defined. The same applies to 
> tooltip-mode,
> scroll-bar-mode and tool-bar-mode. What you are doing looks more 
> like
> configuration code, where you want to make sure that a .emacs 
> works on
> older versions of Emacs.

Thansk, didn't know, I'll correct it.


Nicolas



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

* Re: ELPA: New package: nano-theme
  2021-09-28 17:57   ` Nicolas P. Rougier (inria)
@ 2021-09-28 18:32     ` Stefan Kangas
  2021-09-28 18:48       ` Stefan Monnier
  2021-09-29  5:13       ` Nicolas P. Rougier (inria)
  2021-09-28 18:37     ` Philip Kaludercic
  1 sibling, 2 replies; 16+ messages in thread
From: Stefan Kangas @ 2021-09-28 18:32 UTC (permalink / raw)
  To: Nicolas P. Rougier (inria), Philip Kaludercic; +Cc: emacs-devel

"Nicolas P. Rougier (inria)" <nicolas.rougier@inria.fr> writes:

> Maybe I need to read the documentation on what a theme can
> set. For example, I set the face for the minibuffer (0 & 1) and
> echo area (0 & 1) and I wasn't sure how to specify this in a
> theme. Same for underline to be set at descent line, etc.

You just put the theme in two different files, and set anything you like
from that file.  You could take a look at how modus-themes does this.

> I agree this might be unconventional but since you can specify a
> font family for each face, why not use it? And user can still
> choose its own font stack.

When I switch between themes, I don't want them to have different fonts,
and have to configure it separately for each one.

I have set a font I like in my configuration, and I would like themes to
respect that setting.

>> - Lines 363ff. seem to be indented unconventionally. Maybe add a
>>   .dir-locals.el to make sure everyone is using the same
>>   whitespace
>>   configuration.
>
> Not sure what you mean by add a .dir-locals.el.
> Can you give me a pointer to the documentation?

Search for "directory local variables" in the manual and you will find
instructions there.  Or type `M-x add-dir-local-variable RET'.



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

* Re: ELPA: New package: nano-theme
  2021-09-28 17:57   ` Nicolas P. Rougier (inria)
  2021-09-28 18:32     ` Stefan Kangas
@ 2021-09-28 18:37     ` Philip Kaludercic
  2021-09-29  5:07       ` Nicolas P. Rougier (inria)
  1 sibling, 1 reply; 16+ messages in thread
From: Philip Kaludercic @ 2021-09-28 18:37 UTC (permalink / raw)
  To: Nicolas P. Rougier (inria); +Cc: emacs-devel

"Nicolas P. Rougier (inria)" <nicolas.rougier@inria.fr> writes:

> Many thanks for the review and the feedback.
>
> Philip Kaludercic <philipk@posteo.net> writes:
>
>> "Nicolas P. Rougier (inria)" <nicolas.rougier@inria.fr> writes:
>
>> I see that a lot of faces inherit from nano-default, nano-strong,
>> nano-faded, etc. Wouldn't it make more sense to inherit from
>> built-in
>> faces and then let default, bold, shadow, etc. inherit from these
>> faces.
>
> Having such names helps a lot when you assign faces. For example, when
> I define font-lock-comment-face, I want to make it faded and it feels
> more natural and explicit to inherit from nano-faded. Also, is there
> an associated computational cost when defining a new face?

There has to be *some* computation, but thinking again about this, it
probably shouldn't matter much if you define bold and let everything
inherit from bold or if you define nano-strong and inherit from
nano-strong.

>> Furthermore, I wonder why you define the commands nano-light and
>> nano-dark, instead of two themes, nano-light and nano-dark (along
>> the
>> same lines of what modus-themes currently does). You could also turn
>> nano-setup into a custom theme, so that the user can easily enable
>> and
>> disable it.
>
> Maybe I need to read the documentation on what a theme can set. For
> example, I set the face for the minibuffer (0 & 1) and echo area (0 &
> 1) and I wasn't sure how to specify this in a theme. Same for
> underline to be set at descent line, etc.

Not directly, but you could define a custom option/minor mode that is
activated as part of the theme to change these things too.

>> A few more points:
>>
>> - Should the font really be part of the face? I think it makes sense
>> to
>>   recommend a few fonts, but it is unconventional to add it as
>> part of a
>>   theme.
>
> I agree this might be unconventional but since you can specify a font
> family for each face, why not use it? And user can still choose its
> own font stack.

It was always my impression that faces are for users to set.

>> - When you are only using one branch of a if expression, prefer
>>   when. Instead of (if (not ...) ...), prefer unless.
>
> Thanks, I'll do that.
>
>
>> - Lines 363ff. seem to be indented unconventionally. Maybe add a
>>   .dir-locals.el to make sure everyone is using the same
>> whitespace
>>   configuration.
>
> Not sure what you mean by add a .dir-locals.el.
> Can you give me a pointer to the documentation?

See (emacs) Directory Variables.

>> Also, as you depend on at least 27.1, you probably don't have to
>> check
>> if custom-theme-load-path is defined. The same applies to
>> tooltip-mode,
>> scroll-bar-mode and tool-bar-mode. What you are doing looks more
>> like
>> configuration code, where you want to make sure that a .emacs works
>> on
>> older versions of Emacs.
>
> Thansk, didn't know, I'll correct it.

-- 
	Philip Kaludercic



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

* Re: ELPA: New package: nano-theme
  2021-09-28 18:32     ` Stefan Kangas
@ 2021-09-28 18:48       ` Stefan Monnier
  2021-09-29 15:20         ` Stefan Kangas
  2021-09-29  5:13       ` Nicolas P. Rougier (inria)
  1 sibling, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2021-09-28 18:48 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Nicolas P. Rougier (inria), Philip Kaludercic, emacs-devel

> When I switch between themes, I don't want them to have different fonts,
> and have to configure it separately for each one.
>
> I have set a font I like in my configuration, and I would like themes to
> respect that setting.

Hmm... Custom's "user" theme should take precedence over those other
themes, so you should be able to specify your favorite font in your user
theme and then not be affected by those other theme's choice of font.

Of course, that depends if those choices are applied to the same faces
or not etc... but my point is that you might consider this as a bug that
can be fixed.


        Stefan




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

* Re: ELPA: New package: nano-theme
  2021-09-28 18:37     ` Philip Kaludercic
@ 2021-09-29  5:07       ` Nicolas P. Rougier (inria)
  2021-09-29  6:24         ` Philip Kaludercic
  0 siblings, 1 reply; 16+ messages in thread
From: Nicolas P. Rougier (inria) @ 2021-09-29  5:07 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel


>>> Furthermore, I wonder why you define the commands nano-light 
>>> and
>>> nano-dark, instead of two themes, nano-light and nano-dark 
>>> (along
>>> the
>>> same lines of what modus-themes currently does). You could 
>>> also turn
>>> nano-setup into a custom theme, so that the user can easily 
>>> enable
>>> and
>>> disable it.
>>
>> Maybe I need to read the documentation on what a theme can 
>> set. For
>> example, I set the face for the minibuffer (0 & 1) and echo 
>> area (0 &
>> 1) and I wasn't sure how to specify this in a theme. Same for
>> underline to be set at descent line, etc.
>
> Not directly, but you could define a custom option/minor mode 
> that is
> activated as part of the theme to change these things too.

But then you need to load the theme and activate the minor mode, 
do you?. Having a (nano-light) command is more direct in my 
opinion and in any case, user is free to only load the theme the 
regular way.

>>> - Lines 363ff. seem to be indented unconventionally. Maybe add 
>>> a
>>>   .dir-locals.el to make sure everyone is using the same
>>> whitespace
>>>   configuration.

I had a mix of space and tabs for unknown reason. Hopefully it has 
been fixed.

Nicolas



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

* Re: ELPA: New package: nano-theme
  2021-09-28 18:32     ` Stefan Kangas
  2021-09-28 18:48       ` Stefan Monnier
@ 2021-09-29  5:13       ` Nicolas P. Rougier (inria)
  2021-09-29 12:18         ` Stefan Monnier
  1 sibling, 1 reply; 16+ messages in thread
From: Nicolas P. Rougier (inria) @ 2021-09-29  5:13 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Philip Kaludercic, emacs-devel


Stefan Kangas <stefankangas@gmail.com> writes:

> "Nicolas P. Rougier (inria)" <nicolas.rougier@inria.fr> writes:
>
>> Maybe I need to read the documentation on what a theme can
>> set. For example, I set the face for the minibuffer (0 & 1) and
>> echo area (0 & 1) and I wasn't sure how to specify this in a
>> theme. Same for underline to be set at descent line, etc.
>
> You just put the theme in two different files, and set anything 
> you like
> from that file.  You could take a look at how modus-themes does 
> this.

One of the reason I defined the theme the way it is defined is to 
be able to have both the light and dark theme as for example in 
this screenshot: 
https://github.com/rougier/nano-sidebar/blob/master/images/nano-sidebar-ibuffer.png

I search for how to apply a theme to a single frame but did not 
find any obvious way. I ended up exploiting the dark/light frame 
background mode. As for splitting the theme in two files, I'm not 
sure I understand how this would help with settings some 
variables. 

>>> - Lines 363ff. seem to be indented unconventionally. Maybe add 
>>> a
>>>   .dir-locals.el to make sure everyone is using the same
>>>   whitespace
>>>   configuration.
>>
>> Not sure what you mean by add a .dir-locals.el.
>> Can you give me a pointer to the documentation?
>
> Search for "directory local variables" in the manual and you 
> will find
> instructions there.  Or type `M-x add-dir-local-variable RET'.

In the end, I fixed the indentation that was mixing tabs and 
space.

Nicolas



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

* Re: ELPA: New package: nano-theme
  2021-09-29  5:07       ` Nicolas P. Rougier (inria)
@ 2021-09-29  6:24         ` Philip Kaludercic
  0 siblings, 0 replies; 16+ messages in thread
From: Philip Kaludercic @ 2021-09-29  6:24 UTC (permalink / raw)
  To: Nicolas P. Rougier (inria); +Cc: emacs-devel

"Nicolas P. Rougier (inria)" <nicolas.rougier@inria.fr> writes:

>>>> Furthermore, I wonder why you define the commands nano-light and
>>>> nano-dark, instead of two themes, nano-light and nano-dark (along
>>>> the
>>>> same lines of what modus-themes currently does). You could also
>>>> turn
>>>> nano-setup into a custom theme, so that the user can easily enable
>>>> and
>>>> disable it.
>>>
>>> Maybe I need to read the documentation on what a theme can set. For
>>> example, I set the face for the minibuffer (0 & 1) and echo area (0
>>> &
>>> 1) and I wasn't sure how to specify this in a theme. Same for
>>> underline to be set at descent line, etc.
>>
>> Not directly, but you could define a custom option/minor mode that
>> is
>> activated as part of the theme to change these things too.
>
> But then you need to load the theme and activate the minor mode, do
> you?. Having a (nano-light) command is more direct in my opinion and
> in any case, user is free to only load the theme the regular way.

No, a custom theme can enable a custom option (via
custom-theme-set-variables), and since every minor mode is also a user
option, you can enable a minor mode as part of a theme.

>>>> - Lines 363ff. seem to be indented unconventionally. Maybe add a
>>>>   .dir-locals.el to make sure everyone is using the same
>>>> whitespace
>>>>   configuration.
>
> I had a mix of space and tabs for unknown reason. Hopefully it has
> been fixed.
>
> Nicolas
>

-- 
	Philip Kaludercic



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

* Re: ELPA: New package: nano-theme
  2021-09-29  5:13       ` Nicolas P. Rougier (inria)
@ 2021-09-29 12:18         ` Stefan Monnier
  2021-09-30  7:21           ` Nicolas P. Rougier (inria)
  0 siblings, 1 reply; 16+ messages in thread
From: Stefan Monnier @ 2021-09-29 12:18 UTC (permalink / raw)
  To: Nicolas P. Rougier (inria); +Cc: Stefan Kangas, Philip Kaludercic, emacs-devel

> I search for how to apply a theme to a single frame but did not find any
> obvious way.

Custom themes are sets of user settings (i.e. affecting typically
variables and faces), so they're naturally global.

So you can't have that "single frame" effect directly by specifying face
settings in the theme.

What you can do instead is:
- You can have a custom theme set a particular minor mode (provided as
  part of the theme, for example) and arrange for that minor mode to
  affect faces in a single frame.
- You can use the DISPLAY spec of face settings to restrict your face
  settings to specific classes of frames.  The DISPLAY specs could be
  extended to be more flexible if needed (file a bug report for that).


        Stefan




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

* Re: ELPA: New package: nano-theme
  2021-09-28 18:48       ` Stefan Monnier
@ 2021-09-29 15:20         ` Stefan Kangas
  2021-09-29 16:39           ` Stefan Monnier
  0 siblings, 1 reply; 16+ messages in thread
From: Stefan Kangas @ 2021-09-29 15:20 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Philip Kaludercic, Nicolas P. Rougier (inria), emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Hmm... Custom's "user" theme should take precedence over those other
> themes, so you should be able to specify your favorite font in your user
> theme and then not be affected by those other theme's choice of font.
>
> Of course, that depends if those choices are applied to the same faces
> or not etc... but my point is that you might consider this as a bug that
> can be fixed.

I'm not sure I understand what you mean by "user" theme here.  Do you
mean customizations of faces made with M-x customize-face?

For the default font, I simply do this, on recommendation from `(emacs)
Fonts':

    (add-to-list 'default-frame-alist '(font . "Ubuntu Mono-14"))



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

* Re: ELPA: New package: nano-theme
  2021-09-29 15:20         ` Stefan Kangas
@ 2021-09-29 16:39           ` Stefan Monnier
  0 siblings, 0 replies; 16+ messages in thread
From: Stefan Monnier @ 2021-09-29 16:39 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Nicolas P. Rougier (inria), Philip Kaludercic, emacs-devel

Stefan Kangas [2021-09-29 08:20:13] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Hmm... Custom's "user" theme should take precedence over those other
>> themes, so you should be able to specify your favorite font in your user
>> theme and then not be affected by those other theme's choice of font.
>> Of course, that depends if those choices are applied to the same faces
>> or not etc... but my point is that you might consider this as a bug that
>> can be fixed.
> I'm not sure I understand what you mean by "user" theme here.  Do you
> mean customizations of faces made with M-x customize-face?

Yes.  All modifications done via Custom but "outside of a theme" are
actually internally performed by tweaking a special "user" theme, which
is then combines with the other activated themes.

> For the default font, I simply do this, on recommendation from `(emacs)
> Fonts':
>
>     (add-to-list 'default-frame-alist '(font . "Ubuntu Mono-14"))

That indeed won't interact well with Custom themes.


        Stefan




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

* Re: ELPA: New package: nano-theme
  2021-09-29 12:18         ` Stefan Monnier
@ 2021-09-30  7:21           ` Nicolas P. Rougier (inria)
  0 siblings, 0 replies; 16+ messages in thread
From: Nicolas P. Rougier (inria) @ 2021-09-30  7:21 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Philip Kaludercic, Stefan Kangas, emacs-devel


I think it is possible by first setting the frame-background-mode 
(to 'dark or 'light in my case), then create a new frame and call 
frame-set-background-mode on the newly created frame. It seems to 
work consistently on my system but maybe this is a side 
effect. Here is the code:

#+BEGIN_SRC elisp
(defun nano-new-frame (&optional mode)
  (interactive)
  (let ((mode (or mode (frame-parameter nil 'background-mode)))
        (background-mode frame-background-mode)
        (selected-frame (selected-frame))
        (new-frame nil))

    ;; Set mode
    (setq frame-background-mode mode)
    (setq new-frame (make-frame-command))
    (select-frame new-frame)

    ;; This forces recomputation of faces on the new frame
    (frame-set-background-mode (selected-frame))
           
    (when (eq mode 'light)
      (set-foreground-color nano-light-foreground)
      (set-background-color nano-light-background))

    (when (eq mode 'dark)
      (set-foreground-color nano-dark-foreground)
      (set-background-color nano-dark-background))

    ;; Restore background mode
    (setq frame-background-mode background-mode)
    (frame-set-background-mode selected-frame)
    
    new-frame))
#+END_SRC

Nicolas

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I search for how to apply a theme to a single frame but did not 
>> find any
>> obvious way.
>
> Custom themes are sets of user settings (i.e. affecting 
> typically
> variables and faces), so they're naturally global.
>
> So you can't have that "single frame" effect directly by 
> specifying face
> settings in the theme.
>
> What you can do instead is:
> - You can have a custom theme set a particular minor mode 
> (provided as
>   part of the theme, for example) and arrange for that minor 
>   mode to
>   affect faces in a single frame.
> - You can use the DISPLAY spec of face settings to restrict your 
> face
>   settings to specific classes of frames.  The DISPLAY specs 
>   could be
>   extended to be more flexible if needed (file a bug report for 
>   that).
>
>
>         Stefan




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

* Re: ELPA: New package: nano-theme
  2021-09-28 17:47 ` Philip Kaludercic
  2021-09-28 17:57   ` Nicolas P. Rougier (inria)
@ 2021-10-01  9:59   ` Philip Kaludercic
  2021-10-01 10:05     ` Nicolas P. Rougier (inria)
  1 sibling, 1 reply; 16+ messages in thread
From: Philip Kaludercic @ 2021-10-01  9:59 UTC (permalink / raw)
  To: Nicolas P. Rougier (inria); +Cc: emacs-devel

Philip Kaludercic <philipk@posteo.net> writes:

> "Nicolas P. Rougier (inria)" <nicolas.rougier@inria.fr> writes:
>
>> Dear all,
>>
>> I would like to submit a new package to ELPA which is a theme based on
>> Material colors (https://material.io/) for the light version and Nord
>> colors (https://www.nordtheme.com/) for the dark version. The theme
>> itself is made of (only) 6 different faces and the rationale is here:
>> https://arxiv.org/abs/2008.06030
>>
>> The theme is hosted at https://github.com/rougier/nano-theme and the
>> README displays what it looks like. Note that the theme exploits face
>> inheritance a lot and Eli explained me that it might degrade
>> performances. I don't know if this needs to be fixed.
>
> I see that a lot of faces inherit from nano-default, nano-strong,
> nano-faded, etc. Wouldn't it make more sense to inherit from built-in
> faces and then let default, bold, shadow, etc. inherit from these faces.
>
> Furthermore, I wonder why you define the commands nano-light and
> nano-dark, instead of two themes, nano-light and nano-dark (along the
> same lines of what modus-themes currently does). You could also turn
> nano-setup into a custom theme, so that the user can easily enable and
> disable it.

I see nano-theme was added despite still providing functions for themes,
instead of using customize (and a few other points I raised). Have I
missed an explanation as to why this decision was made? Did it turn out
to not be possible? It seems regrettable to provide a theme on ELPA that
evades the conventional theme infrastructure, especially considering
that ELPA currently doesn't provide that many themes to begin with.

> A few more points:
>
> - Should the font really be part of the face? I think it makes sense to
>   recommend a few fonts, but it is unconventional to add it as part of a
>   theme.
> - When you are only using one branch of a if expression, prefer
>   when. Instead of (if (not ...) ...), prefer unless.
> - Lines 363ff. seem to be indented unconventionally. Maybe add a
>   .dir-locals.el to make sure everyone is using the same whitespace
>   configuration.
>
> Also, as you depend on at least 27.1, you probably don't have to check
> if custom-theme-load-path is defined. The same applies to tooltip-mode,
> scroll-bar-mode and tool-bar-mode. What you are doing looks more like
> configuration code, where you want to make sure that a .emacs works on
> older versions of Emacs.
>
>> Best,
>> Nicolas

-- 
	Philip Kaludercic



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

* Re: ELPA: New package: nano-theme
  2021-10-01  9:59   ` Philip Kaludercic
@ 2021-10-01 10:05     ` Nicolas P. Rougier (inria)
  2021-10-01 10:15       ` Philip Kaludercic
  0 siblings, 1 reply; 16+ messages in thread
From: Nicolas P. Rougier (inria) @ 2021-10-01 10:05 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel


>
> I see nano-theme was added despite still providing functions for 
> themes,
> instead of using customize (and a few other points I 
> raised). Have I
> missed an explanation as to why this decision was made? Did it 
> turn out
> to not be possible? It seems regrettable to provide a theme on 
> ELPA that
> evades the conventional theme infrastructure, especially 
> considering
> that ELPA currently doesn't provide that many themes to begin 
> with.

I cannot speak for decision but I intend to address all your 
comments for next version (next week most probably).

Nicolas



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

* Re: ELPA: New package: nano-theme
  2021-10-01 10:05     ` Nicolas P. Rougier (inria)
@ 2021-10-01 10:15       ` Philip Kaludercic
  0 siblings, 0 replies; 16+ messages in thread
From: Philip Kaludercic @ 2021-10-01 10:15 UTC (permalink / raw)
  To: Nicolas P. Rougier (inria); +Cc: emacs-devel

"Nicolas P. Rougier (inria)" <nicolas.rougier@inria.fr> writes:

> I cannot speak for decision but I intend to address all your comments
> for next version (next week most probably).

Ok, that is good to hear. I was just surprised to see the package in
elpa.git all of a sudden.

-- 
	Philip Kaludercic



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

end of thread, other threads:[~2021-10-01 10:15 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-28 13:11 ELPA: New package: nano-theme Nicolas P. Rougier (inria)
2021-09-28 17:47 ` Philip Kaludercic
2021-09-28 17:57   ` Nicolas P. Rougier (inria)
2021-09-28 18:32     ` Stefan Kangas
2021-09-28 18:48       ` Stefan Monnier
2021-09-29 15:20         ` Stefan Kangas
2021-09-29 16:39           ` Stefan Monnier
2021-09-29  5:13       ` Nicolas P. Rougier (inria)
2021-09-29 12:18         ` Stefan Monnier
2021-09-30  7:21           ` Nicolas P. Rougier (inria)
2021-09-28 18:37     ` Philip Kaludercic
2021-09-29  5:07       ` Nicolas P. Rougier (inria)
2021-09-29  6:24         ` Philip Kaludercic
2021-10-01  9:59   ` Philip Kaludercic
2021-10-01 10:05     ` Nicolas P. Rougier (inria)
2021-10-01 10:15       ` Philip Kaludercic

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