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.

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

Use let* instead of let #96

Open
gongzhitaao opened this issue Mar 15, 2020 · 2 comments
Open

Use let* instead of let #96

gongzhitaao opened this issue Mar 15, 2020 · 2 comments

Comments

@gongzhitaao
Copy link

https://github.com/arcticicestudio/nord-emacs/blob/d828752e270978a56bde19986c98b1bbe8f51386/nord-theme.el#L101

It would be much easier and less error-prone if we use let* instead of let. let* allows to use the variable defined before the current line. For example:

(let* ((class '((class color) (min-colors 89)))
  ;; omit lines
  (nord12 (if (nord-display-truecolor-or-graphic-p) "#D08770" "brightyellow"))
  ;; omit lines
  ;; (nord-annotation (if (nord-display-truecolor-or-graphic-p) "#D08770 "brightyellow")) <-- original
  (nord-annotation (if (nord-display-truecolor-or-graphic-p) nord12 "brightyellow"))
@arcticicestudio
Copy link
Contributor

Hi @gongzhitaao 👋, thanks for your contribution 👍

That sounds like a good and inexpensive improvement 😄
Unfortunately I'm not really familiar with Lisp at all so I'll need to check and test if there might be problems when it comes to the special Emacs Lisp syntax derivations.
Maybe someone with more experience post some feedback or some examples/links if it is used in other popular themes too without any disadvantages.

@gongzhitaao
Copy link
Author

gongzhitaao commented Apr 8, 2020

Some examples:

The only different between let* and let is let* allows you to use variables previously defined.

@arcticicestudio arcticicestudio added the Hacktoberfest This repository participates in the Hacktoberfest label Oct 8, 2020
@arcticicestudio arcticicestudio removed the Hacktoberfest This repository participates in the Hacktoberfest label Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants