Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Invalid face box error #131

Merged
merged 1 commit into from
Mar 9, 2025
Merged

Fix Invalid face box error #131

merged 1 commit into from
Mar 9, 2025

Conversation

cariandrum22
Copy link
Contributor

The style attribute sunken-button for the :box attribute set on a button or custom-button* is not a valid value1.

This was not made manifest by a bug2 on the emacs side but is expected to be fixed and become an error in the version 30 release.

In the context of the code, sunken-button itself is considered equivalent to pressed-button, so all places where sunken-button is specified have been replaced with pressed-button.

@cariandrum22
Fix Invalid face box error (nordtheme#130)
e6977cf 
The `style` attribute `sunken-button` for the `:box` attribute set on a
`button` or `custom-button*` is not a valid value[1].

This was not made manifest by a bug[2] on the emacs side but is expected
to be fixed and become an error in the version 30 release.

In the context of the code, `sunken-button` itself is considered
equivalent to `pressed-button`, so all places where `sunken-button` is
specified have been replaced with `pressed-button`.

[1]: https://www.gnu.org/software/emacs/manual/html_node/elisp/Face-Attributes.html
[2]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=67404
@cariandrum22 cariandrum22 changed the title Fix Invalid face box error (#130) Fix Invalid face box error Feb 18, 2024
@amasover
Copy link

I tested this change with Quelpa, just pointing to your branch instead of nordtheme/emacs, and everything seems to be working fine.

Now that Emacs 30 is officially released (now even 30.1), Nord Theme is broken on Emacs without this PR.

Gasahorlogo added a commit to Gasahorlogo/emacs-config that referenced this pull request Mar 2, 2025
@Gasahorlogo
Fix: nord-theme: Invalid face box error #131, see also: nordtheme/ema…
6f6c698 
…cs#131
kousiknandy
kousiknandy reviewed Mar 4, 2025
View reviewed changes
Copy link

@kousiknandy kousiknandy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirming it works

@svengreb svengreb mentioned this pull request Mar 9, 2025
Closed
@svengreb svengreb self-assigned this Mar 9, 2025
@svengreb svengreb added this to the version-next milestone Mar 9, 2025
@svengreb svengreb linked an issue Mar 9, 2025 that may be closed by this pull request
"sunken-button" no longer valid style in emacs master #130
Closed
@svengreb svengreb moved this from triage to review in Planning & Roadmaps Mar 9, 2025
@svengreb
Copy link
Member

svengreb commented Mar 9, 2025

@cariandrum22 @amasover @kousiknandy Thank you for your contributions and patience! 🙏🏼
It's been a while since I had free time to focus more on Nord, and my open source projects in general, and invest time in this issue due to work-life balance. Please keep in mind that maintainers must maintain (pun intended 😁) balance for their open source work and life to avoid burnouts so this gap in time has nothing to do with you or your contribution.

I recently published the first "Northern Post — The state and roadmap of Nord" announcement which includes all details about the plans and future of the Nord project, including the goal of catching up with the backlog. This issue is part of the backlog and therefore I want to triage and process it to get one step closer to a "clean state". Read the announcement about reaching the "clean" contribution triage state in Nord's discussions for more details about the goal.

Due to the release of Emacs version 30.1 some weeks ago this problem might affect more users that follow the stable release channel so this issue must be resolved as quickly as possible.
I added this pull request to the central and single-source-of-truth project board for the next release.
I'll quickly review and test the changes to publish a new theme version as soon as possible.

svengreb
svengreb approved these changes Mar 9, 2025
View reviewed changes
Copy link
Member

@svengreb svengreb left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with Emacs version 30.1 on macOS, installed via Homebrew.
The theme can be loaded without errors again.
Thanks again for your contribution 🙌🏼
     🏂
    ❄️
   ❄️
  ❄️
 ❄️
❄️

@svengreb svengreb moved this from review to approved in Planning & Roadmaps Mar 9, 2025
@svengreb svengreb merged commit 336a76a into nordtheme:develop Mar 9, 2025
@github-project-automation github-project-automation bot moved this from approved to completed in Planning & Roadmaps Mar 9, 2025
@svengreb svengreb removed their assignment Mar 9, 2025
@svengreb svengreb mentioned this pull request Mar 9, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kousiknandy kousiknandy kousiknandy left review comments

@svengreb svengreb svengreb approved these changes

Assignees
No one assigned
Labels
context-ui priority-high scope-compatibility scope-stability
Projects
Planning & Roadmaps
Status: completed
Milestone
0.6.0
Development

Successfully merging this pull request may close these issues.

"sunken-button" no longer valid style in emacs master
4 participants
@cariandrum22 @amasover @svengreb @kousiknandy