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

nord-theme leaks unprefixed names to the global environment #66

Closed
slippycheeze opened this issue Sep 13, 2018 · 3 comments · Fixed by #67
Closed

nord-theme leaks unprefixed names to the global environment #66

slippycheeze opened this issue Sep 13, 2018 · 3 comments · Fixed by #67
Labels
priority-high scope-compatibility type-bug
Milestone
0.4.0

Comments

@slippycheeze
Copy link
Contributor

Sadly, Emacs Lisp has no namespaces, so every top level definition is visible to the entire universe. That includes variables and functions.

The current nord-theme defines one of each, nord-brightened-comments and brightened-comment-color, which are leaked.

The former is ... ok, though not best practice. The later has a chance of conflicting with anything else that uses a similar global name, and is definitely not in line with best practices for Emacs code.
slippycheeze pushed a commit to slippycheeze/nord-emacs that referenced this issue Sep 13, 2018
Prefix internal names to avoid global namespace pollution
c7f574e 
Sadly, Emacs Lisp has no namespaces, so every top level definition is
visible to the entire universe.  That includes variables and
functions.

This commit prefixes them to avoid "leaking" them in a way that could
conflict with other code.  The convention of `package--` is used to
mirror the most current Emacs coding conventions.

There are no functional changes from this modification.

Closes nordtheme#66
@slippycheeze slippycheeze mentioned this issue Sep 13, 2018
Merged
@arcticicestudio arcticicestudio added this to the 0.4.0 milestone Sep 13, 2018
@arcticicestudio
Copy link
Contributor

Thanks for your contribution 👍

That's another point for the „today I learned…“ list!
I've never coded something in (Emacs) Lisp before except this theme so this is a nice info for latter implementations.

@slippycheeze
Copy link
Contributor Author

@arcticicestudio, thank you so much for the work! I have a few more things on my list to help improve, one key one being at the heart of #59, which I'll submit the same way ... hopefully in the next few days, depending. :)

@arcticicestudio
Copy link
Contributor

That's awesome, I'm happy about any help! It bothers me a lot that I could not solve some of the open issues in this repository due to the missing experience with Emacs and the fact that even after searching through many documentations and tutorials not able to find a up-to-date or correct solution. There are also not always ways to reproduce some of the problems.

In short: OSS lives from contributors and I welcome any help for this very much 😄

@arcticicestudio arcticicestudio mentioned this issue Dec 16, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
priority-high scope-compatibility type-bug
Projects
None yet
Milestone
0.4.0
Development

Successfully merging a pull request may close this issue.

Prefix internal names to avoid global namespace pollution slippycheeze/nord-emacs
2 participants
@slippycheeze @arcticicestudio