rougier / nano-modeline

GNU Emacs / N Λ N O Modeline
GNU General Public License v3.0
170 stars 29 forks source link

Latest master branch commits break invalid face box atrributes. #67

Closed artelse closed 4 months ago

artelse commented 9 months ago

The latest emacs 30.0.5 master branch commits made changes to the validation logic for the :box attribute:

* src/xfaces.c (Finternal_set_lisp_face_attribute): Fix the logic
of validating the :box attribute.  The previous code would always
allow invalid attributes of :box as long as the invalid attribute
was the last in the list.  (Bug#67404)

And:

* src/xfaces.c (Finternal_set_lisp_face_attribute): Fix validation
of the :style attribute of :box.  Previously, nil for :style was not
accepted, which causes 'mode-line-inactive' face to be rejected.
(Bug#67567)

Likely an update is needed how nano-modeline sets some face attributes, so I changed :style none on line 164 and 171 to read :style nil. (none is an invalid attribute) When I eval/load the package it works as expected, but when loaded in my init, it craps out with no error message. (..)

This was the error I got previously:

Debugger entered--Lisp error: (error "Invalid face box" :line-width 2 :color "black" :style none)
  set-face-attribute(nano-modeline-button-active-face #<frame *scratch* - GNU Emacs at lae 0x55f0d0222328> :foreground "black" :background "white" :family "Roboto Mono" :box (:line-width 2 :color "black" :style none))
  face-spec-set-2(nano-modeline-button-active-face #<frame *scratch* - GNU Emacs at lae 0x55f0d0222328> (:foreground "black" :background "white" :family "Roboto Mono" :box (:line-width 2 :color "black" :style none)))
  face-spec-recalc(nano-modeline-button-active-face #<frame *scratch* - GNU Emacs at lae 0x55f0d0222328>)
  face-spec-set(nano-modeline-button-active-face ((t :foreground "black" :background "white" :family "Roboto Mono" :box (:line-width 2 :color "black" :style none))) face-defface-spec)
  custom-declare-face(nano-modeline-button-active-face ((t :foreground "black" :background "white" :family "Roboto Mono" :box (:line-width 2 :color "black" :style none))) "Active button face")
  require(nano-modeline nil t)
rougier commented 9 months ago

I do not have 30 installed but from the error message it seems there are other none that are set somewhere. Did you search the whole nano-modeline.el file and possibly your custom settings?

artelse commented 9 months ago

yes, searched packages, but no other :box and :style attributes found that could cause problems. Digged in more, but haven't found what causes it to fail on init. Perhaps just wait for an update to 30, also had problems with xwidget that don't work any longer (xwidget's fault, not emacs).

larstvei commented 8 months ago

I have the same issue.

It seems like :style none causes the error. Commenting out these two:

From the docs:

The value style specifies whether to draw a 3D box. If it is released-button, the box looks like a 3D button that is not being pressed. If it is pressed-button, the box looks like a 3D button that is being pressed. If it is nil, flat-button or omitted, a plain 2D box is used.

I am guessing that recent versions of Emacs enforces these values. My guess is that simply removing the :style would have the same effect as none.

artelse commented 8 months ago

I am guessing that recent versions of Emacs enforces these values. My guess is that simply removing the :style would have the same effect as none.

If removing the attribute works, it would be the best fix with backwards compatibility in mind. Edit: works fine!

aaronjensen commented 8 months ago

There are further problems with Emacs 30 having to do with faces. I reported bug#67991 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=67991

Though it may require a change in nano-modeline, because nano-modeline may have been relying on buggy behavior to begin with.

rougier commented 8 months ago

@aaronjensen Thanks for the bug report. From what I understood, this means we have to add defvar for each and every local variables in the modeline eval? Can you open a specific issue for that?

aaronjensen commented 8 months ago

It's possible. I wasn't aware of others, so I don't know what I'd put in the issue. I currently don't have any problems taht I know of.

cmacrae commented 5 months ago

@rougier I see these issues have since been addressed, exist on the master branch, and have been noted in NEWS.
would you mind pushing a new version to ELPA so we can benefit? :)

rougier commented 4 months ago

Done!