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

Make OneNord follow background option as it's set #30

Merged
merged 1 commit into from
Jan 6, 2022

Conversation

Slotos
Copy link
Contributor

@Slotos Slotos commented Jan 6, 2022

Before this change, the option was followed once
on initial load due to require behaviour.

Additionally, theme option is now respected
on repeated loads. E.g.:

:lua require('onenord').setup({theme = 'dark'})
:lua require('onenord').setup({theme = 'light'})

Yields a light variant, as you'd expect.

In order to unset forced theme, set it to false.
This is due to the fact that {a = nil} and {}
are equivalent. As such, providing theme = nil
is equivalent to not providing it at all.

@Slotos
Copy link
Contributor Author

Slotos commented Jan 6, 2022

Forced push fixes StyLua style violation.

Copy link
Owner

@rmehri01 rmehri01 left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the PR, I think it looks good! Do you mind adding BREAKING CHANGE: or fix!: in front of your commit message just so people know when they upgrade?

Before this change, the option was followed once
on initial load due to `require` behaviour.

Additionally, theme option is now respected
on repeated loads. E.g.:

```
:lua require('onenord').setup({theme = 'dark'})
:lua require('onenord').setup({theme = 'light'})
```

Yields a light variant, as you'd expect.

In order to unset forced theme, set it to `false`.
This is due to the fact that `{a = nil}` and `{}`
are equivalent. As such, providing `theme = nil`
is equivalent to not providing it at all.
@Slotos
Copy link
Contributor Author

Slotos commented Jan 6, 2022

Added breaking change, as it's not really a fix, but a change that might break some workflows.

I also removed TODO: part, replacing it with explanation about why theme = nil doesn't reset theme when calling setup.

@rmehri01
Copy link
Owner

rmehri01 commented Jan 6, 2022

I see, thanks again!

@rmehri01 rmehri01 merged commit 1590c43 into rmehri01:main Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants