-
-
Notifications
You must be signed in to change notification settings - Fork 631
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
Fix error loading non-existant themes directory and use default themes as the base when merging #2411
Conversation
Tagging @tlinford to take a look as he also encountered this problem (or a similar one?) |
Problem is fixed with these changes. |
Thanks @tlinford - now that we know this is the same issue, I'm going to tag @jaeheonji for this. What do you think about these changes? |
@imsnif @Imberflur But, The only question is, what are the advantages of not providing a default theme via If the same theme exists, there will be no problem because the user's theme will be prioritized. If there is a case where |
(FWIW I'm not familiar at all with the usage of
I was imaging that the default theme would be provided but the user's theme(s) (from the themes directory) would not be loaded if |
edit: I investigated more and tried it out. Seems like |
@Imberflur Oh I see, it make sense.
Sure, I totally agree with this idea. |
@jaeheonji I pushed a commit that should implement this |
If the themes directory is derived from the config directory (rather than being specified explicitly in the config_options), we will avoid trying to load from it if it doesn't exist.
the config. This avoids the default themes overriding themes specified in the config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [aquaproj/aqua-registry](https://togithub.com/aquaproj/aqua-registry) | minor | `v4.19.0` -> `v4.20.0` | | [zellij-org/zellij](https://togithub.com/zellij-org/zellij) | minor | `v0.36.0` -> `v0.37.0` | --- ### Release Notes <details> <summary>aquaproj/aqua-registry</summary> ### [`v4.20.0`](https://togithub.com/aquaproj/aqua-registry/releases/tag/v4.20.0) [Compare Source](https://togithub.com/aquaproj/aqua-registry/compare/v4.19.0...v4.20.0) [Issues](https://togithub.com/aquaproj/aqua-registry/issues?q=is%3Aissue+milestone%3Av4.20.0) | [Pull Requests](https://togithub.com/aquaproj/aqua-registry/pulls?q=is%3Apr+milestone%3Av4.20.0) | aquaproj/aqua-registry@v4.19.0...v4.20.0 #### 🎉 New Packages [#​13132](https://togithub.com/aquaproj/aqua-registry/issues/13132) [segmentio/golines](https://togithub.com/segmentio/golines): A golang formatter that fixes long lines [@​iwata](https://togithub.com/iwata) #### Fixes [#​13143](https://togithub.com/aquaproj/aqua-registry/issues/13143) [terraform-linters/tflint](https://togithub.com/terraform-linters/tflint): Support old versions </details> <details> <summary>zellij-org/zellij</summary> ### [`v0.37.0`](https://togithub.com/zellij-org/zellij/releases/tag/v0.37.0) [Compare Source](https://togithub.com/zellij-org/zellij/compare/v0.36.0...v0.37.0) In this release we've done a lot of work on our WebAssembly / WASI plugin system and are very excited to invite adventurous Rust developers to come pioneer our plugin system with us. To read more, please see the official [Plugin Documentation](https://zellij.dev/documentation/plugins.html). Please also drop by our Discord or Matrix and show us the plugins you're working on! #### Other Highlights - Some basic themes are now included with the release, give it a try by starting Zellij with `zellij options --theme catppuccin-mocha` - Layouts now support environment variables and tilde expansions - We can now provide a `--cwd` option when starting Zellij #### All changes - fix(plugin): respect hide session option on compact-bar by [@​pedromfedricci](https://togithub.com/pedromfedricci) in [zellij-org/zellij#2368 - feat: Add layout configuration to exclude panes from tab sync by [@​on3iro](https://togithub.com/on3iro) in [zellij-org/zellij#2314 - Fix 2205 - Support cwd by [@​Kangaxx-0](https://togithub.com/Kangaxx-0) in [zellij-org/zellij#2290 - feat(plugins): reload plugin at runtime by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2372 - Update architecture doc by [@​Kangaxx-0](https://togithub.com/Kangaxx-0) in [zellij-org/zellij#2371 - feat(themes): add nightfox themes by [@​EdenEast](https://togithub.com/EdenEast) in [zellij-org/zellij#2384 - feat: provide default themes by [@​jaeheonji](https://togithub.com/jaeheonji) in [zellij-org/zellij#2307 - feat(plugins): update and render plugins asynchronously by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2410 - feat(layout): Support environment variables in cwd ([#​2288](https://togithub.com/zellij-org/zellij/issues/2288)) by [@​shahamran](https://togithub.com/shahamran) in [zellij-org/zellij#2291 - Add file path context to all IO errors in ConfigError by [@​Imberflur](https://togithub.com/Imberflur) in [zellij-org/zellij#2412 - fix(e2e): fix flaky locked mode test by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2413 - Fix error loading non-existant themes directory and use default themes as the base when merging by [@​Imberflur](https://togithub.com/Imberflur) in [zellij-org/zellij#2411 - improve build/ci times by [@​tlinford](https://togithub.com/tlinford) in [zellij-org/zellij#2396 - Do not unwrap() the sticky bit setting! by [@​valpackett](https://togithub.com/valpackett) in [zellij-org/zellij#2424 - Use rust 1.67 by [@​har7an](https://togithub.com/har7an) in [zellij-org/zellij#2375 - Fix issue 2421 - Update config file output by [@​Kangaxx-0](https://togithub.com/Kangaxx-0) in [zellij-org/zellij#2443 - feat(plugins): Plugin workers and strider by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2449 - fix: cwd of newtab action by [@​onichandame](https://togithub.com/onichandame) in [zellij-org/zellij#2455 - feat(wasm-plugin-system): major overhaul and some goodies by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2510 - feat(plugins): extensive plugin api by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2516 - fix: runtime panic because of local cache by [@​jaeheonji](https://togithub.com/jaeheonji) in [zellij-org/zellij#2522 - fix(output): do not hide cursor on a render that does not include visual assets by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2528 - fix(screen): focus tab as well as pane when launching existing plugin by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2530 - fix(strider): clear search term on ESC by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2531 - fix(plugins): only listen to hd if a plugin is subscribed to hd events by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2529 - fix(logs): suppress debug logs when not debugging by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2532 - fix(plugins): allow loading relative urls by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2539 - feat(plugins): plugin pane state events by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2545 - performance(plugins): use a debounced fs watcher by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2546 - feat(plugins): more plugin api methods by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2550 - refactor(plugins): improve api by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2552 - feat(plugins): strider improvements by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2551 - docs(plugins): document the zellij-tile events and commands api by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2554 - docs(plugins): better zellij-tile-docs by [@​imsnif](https://togithub.com/imsnif) in [zellij-org/zellij#2560 #### New Contributors - [@​on3iro](https://togithub.com/on3iro) made their first contribution in [zellij-org/zellij#2314 - [@​Kangaxx-0](https://togithub.com/Kangaxx-0) made their first contribution in [zellij-org/zellij#2290 - [@​EdenEast](https://togithub.com/EdenEast) made their first contribution in [zellij-org/zellij#2384 - [@​shahamran](https://togithub.com/shahamran) made their first contribution in [zellij-org/zellij#2291 - [@​Imberflur](https://togithub.com/Imberflur) made their first contribution in [zellij-org/zellij#2412 - [@​valpackett](https://togithub.com/valpackett) made their first contribution in [zellij-org/zellij#2424 - [@​onichandame](https://togithub.com/onichandame) made their first contribution in [zellij-org/zellij#2455 **Full Changelog**: zellij-org/zellij@v0.36.0...v0.37.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/scottames/dots). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMjYuMCIsInVwZGF0ZWRJblZlciI6IjM1LjEyNi4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
When I cloned the repo and ran
cargo xtask run
I encountered an issue where thezellij
was trying to load a themes directory at~/.config/zellij/themes
that did not exist.If looks like a recent PR (#2307), removed the
if theme_dir.is_dir() {
that was checking if the theme directory exists before trying to use it. I assume we do want to produce a hard error when the theme dir is specifically specified in the config, so I added an equivalent check that only occurs in the case where the theme dir is not specified in the config.While fixing this, I also noticed that the default themes are applied over the top of the themes specified in the config. Presumably, the opposite is better since it allows overriding the defaults (and is more consistent since the themes from the themes directory are applied on top of everything else and so are able to override the defaults regardless).
One other thing I noticed (but haven't made any changes on), is that there is an option in
Command::Setup
calledclean
that indicates to only use the defaults for the config provided by zellij. I was wondering, if this should also apply to themes, since right now (afaict from the code) zellij will still load themes from the themes directory with this enabled.P.S. I joined the discord server, feel free to contact me there as
@imbris