nano-modeline-faces vs faces

[aaronjensen]

Hi, now that the simpler branch is merged, I've started to try to rebuild my modeline with the new toolkit. I was curious about the reason for nano-modeline-faces vs having separate named faces for each possible face. As a user who is needing to customize, what I have ended up doing is adding my own faces and using them. I've never had to do that before in any other package, so it's a bit cumbersome. Also, the way that the base face gets inherited in is clever, but also a bit surprising. I personally think the old way was more manageable as a consumer of the library, but I may very well be missing something.

[alkurbatov]

Hi, +1 to this issue. While I like the new branch pretty much, previously it was relatively simple to change foreground for secondary elements (line and column numbers), now it requires additional tinkering.

[rougier]

You can still use a direct face when designing a new mode. But the faces indirection make it easier to define active/inactive faces.

[aaronjensen]

There isn't always a direct face. See:

nano-modeline/nano-modeline.el

Line 204 in a18780c

(name-active . (bold))

I'm not sure it makes anything easier, it just adds another layer of abstraction around the faces. It means I need to do two things (add an item to an alist and declare a new face), rather than one (change a face). It also makes is so that nano-modeline cannot be theme'd, because now there is configuration code and custom faces that must be set instead of just defining faces.

Could we have a face for each active/inactive state instead?

[rougier]

Problem is that when configuring the mode line, I need a "dynamic" face for applying active / inactive faces. But I agree it is far from ideal and I'm open to suggestions.

[aaronjensen]

What do you mean by "dynamic"? How is it any different from dynamically constructing the face name, e.g., nano-modeline-name-active-face? Isn't that what the old version did?

[rougier]

for example, this is the code for the buffer name. I just call the nano-modeline-face function that will take care of applying active or inactive face (using nameas a key). How would you write this instead?

(defun nano-modeline-buffer-name (&optional name)
  "Buffer name"
  
  (propertize
   (cond (name name)
         ((buffer-narrowed-p) (format"%s [narrow]" (buffer-name)))
         (t (buffer-name)))
   'face (nano-modeline-face 'name)))

[aaronjensen]

(nano-modeline-face 'name) would resolve to nano-modeline-name-active-face (or inactive). You would declare all permutations of faces.

I feel like I'm missing something here though? The keys aren't dynamic, right? They can't be anything, there's only a fixed set (at least declared by nano-modeline, obviously an end user could decide to add their own faces).

[rougier]

Yes, keys are not dynamic and they can be anything. They are used to find a corresponding face using the nano-modeline-faces and the current state (active / inactive).

Alternative would be to call nano-modeline-face with active-face and inactive face. Or to have a convention for naming faces (e.g. some-face-active some-face-inactive)

[aaronjensen]

Yeah, I think the naming convention makes the most sense. It gives users of the package a way to customize faces they are used to, and power users a utility and convention for additional faces.

Alternatively, you can do both—maintain the map but also declare faces for all of the default faces and states. In other words, rather than using bold, you would have an active name face declared and mapped. I’m just not sure that additional level of indirection buys us anything.

[rougier]

Ok, I'll create a branch to test. Not sure when though...