feat: support themes directory

[jaeheonji]

Resolve #1491

EDIT: The theme_dir option was removed due to another issue. Therefore, themes directory always exists under the zellij config directory.

[jaeheonji]

@a-kenji Hi!

I plan to merge this PR if there are no problems.
Do you have any feedback for this PR?

[a-kenji]

@jaeheonji,
Awesome. I will be home soon, then I will have a look. Why wasn't the theme directory configuration working?

[jaeheonji]

@a-kenji Thank you!

Why wasn't the theme directory configuration working?

When I try to add new options, I get a large_enum_variant error in clippy. It seems that there are several ways to solve it, but this problem seems to belong to a larger category, so I will try to solve it separately.

[a-kenji]

Thanks for implementing this, it is pretty awesome!

There are a few things that can get addressed:

I think the serialization should be done in the setup step itself, that way the themes specified in the config and the themes directory can get merged. And people can have some themes in their config and some in the directory. This will also take some logic outside of the client, and make it possibly less error prone.

What do you think about allowing the name of the theme in the theme itself? That way it would be more portable and you could copy your theme from a theme directory into the configuration file.
In the same vein, could it make sense to allow specifying related themes inside the same file?
For example molokai.yaml having a molokai-dark and a molokai-light.

Also it would be great, if you could add the theme directory to zellij setup --check!

What do you think?

[a-kenji]

@a-kenji Thank you!

Why wasn't the theme directory configuration working?

When I try to add new options, I get a large_enum_variant error in clippy. It seems that there are several ways to solve it, but this problem seems to belong to a larger category, so I will try to solve it separately.

Ah, yes that makes sense.
Depending on how you want to solve it it can be a bigger item.
Boxing, or allowing this particular large enum variant, probably won't have a large negative impact.

[jaeheonji]

I think the serialization should be done in the setup step itself, that way the themes specified in the config and the themes directory can get merged. And people can have some themes in their config and some in the directory. This will also take some logic outside of the client, and make it possibly less error prone.

What do you think about allowing the name of the theme in the theme itself? That way it would be more portable and you could copy your theme from a theme directory into the configuration file.
In the same vein, could it make sense to allow specifying related themes inside the same file?
For example molokai.yaml having a molokai-dark and a molokai-light.

You're right. I initially thought of two ways. The first, as you mentioned, is to merge themes by loading individual files during the setup process. The second is to look up themes in the config first, and load the file if there is no theme in the config.

Both are good methods, but I did not choose the first method because it seems that there will be more File I/O than the second method. (Of course, it usually won't affect performance that much.)

However, if it is an example like molokai, I think the first method is fine. And, it seems that some edge cases can be solved.

Also it would be great, if you could add the theme directory to zellij setup --check!

I agree 100% 👍

Ah, yes that makes sense.
Depending on how you want to solve it it can be a bigger item.
Boxing, or allowing this particular large enum variant, probably won't have a large negative impact.

Cool! It will be helpful.

Thank you so much for the feedback and finding the missing parts. It will take some time to fix 😅

[jaeheonji]

@a-kenji I updated it. please check it out when you have time 😄

[a-kenji]

This is very awesome, and it works now as I expect it! Thanks for the great work.