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

Simplify the conditions at the beginning of the theme #295

Merged
merged 2 commits into from
Apr 16, 2022
Merged

Simplify the conditions at the beginning of the theme #295

merged 2 commits into from
Apr 16, 2022

Conversation

jvoisin
Copy link
Contributor

@jvoisin jvoisin commented Apr 4, 2022

  • Use ternary when possible
  • use get instead of nested if
  • Inline the s:logWarning function since it was only used once
  • Group some block where it made sense

@jvoisin
Simplify the conditions at the beginning of the theme
46b2a59 
- Use ternary when possible
- use `get` instead of nested if
- Inline the s:logWarning function since it was only used once
- Group some block where it made sense
@svengreb
Copy link
Member

svengreb commented Apr 5, 2022

Hi @jvoisin 👋, thanks for your contribution 👍
I'm currently busy with the preparations for the roadmap and future plan of the whole Nord project, but I'll try to review the changes within the next week(s).

svengreb
svengreb approved these changes Apr 16, 2022
View reviewed changes
Copy link
Member

@svengreb svengreb left a comment

Choose a reason for hiding this comment

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

Thanks for the refactoring, this looks way cleaner and clearer!

@svengreb svengreb added this to the version-next milestone Apr 16, 2022
@svengreb svengreb merged commit 291e05d into nordtheme:develop Apr 16, 2022
arcticicestudio pushed a commit that referenced this pull request May 14, 2022
@jvoisin @svengreb
Refactor theme configuration conditions (#295)
bf09e50 
The conditions and default values of the theme configurations were quite
verbose so this commit improves them by...

- ...using inline ternary operators instead of if/else blocks to reduce
  the code overhead and make it way more readable.
- ...using Vim builtin `get` function [1] instead of if/else blocks.
- ...inlining the script-scoped `logWarning` function since it was only
  used once.
- ...grouping some blocks where it made sense.

[1]: https://vimhelp.org/builtin.txt.html#builtin.txt#get%28%29

GH-295


Co-authored-by: Sven Greb <development@svengreb.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arcticicestudio arcticicestudio arcticicestudio approved these changes

@svengreb svengreb svengreb approved these changes

Assignees
No one assigned
Labels
scope-maintainability scope-quality type-improvement
Projects
None yet
Milestone
0.19.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jvoisin @svengreb @arcticicestudio