tinted-theming / tinty

A base16 and base24 color scheme manager
MIT License
43 stars 5 forks source link

Support for emacs '-theme.el' files [Feature request] #30

Closed 1ynnshell closed 3 months ago

1ynnshell commented 3 months ago

Is your feature request related to a problem? Please describe.

It is good practice for emacs themes to end in the stub -theme.el. Current implementation of Tinty does not support this naming convention:

        let theme_dir = fs::read_dir(&themes_path)
            .map_err(anyhow::Error::new)
            .with_context(|| format!("Themes are missing from {}, try running `{} install` or `{} update` and try again.", item.name, REPO_NAME, REPO_NAME))?;
        let theme_option = &theme_dir.filter_map(Result::ok).find(|entry| {
            let path = entry.path();
            let filename = path.file_stem().and_then(|name| name.to_str());
            full_scheme_name == filename.unwrap_or_default()
        });

it checks for exact naming, which works for almost all applications.

Describe the solution you'd like

Implementation of a file-name override in the .toml config which allows for exceptional cases:

[[items]]
name = "base16-emacs"
path = "https://github.com/belak/base16-emacs"
themes-dir="build"
file-override = "$(tinty current)-theme"
hook = "cp -f %f ~/.emacs.d/custom-theme.el && emacsclient -e '(load-theme 'custom t)'"

Describe alternatives you've considered

Rebuilding & packaging the base16-emacs for consumption of a manager, instead of as an emacs package. This would allow foregoing of the -theme.el standard. This is not a desirable fix, but if the above functionality is out of scope of Tinty I can understand.

Additional context

If the suggested implementation is desirable I can work on the feature and test it. I would like to avoid having to fork the existing build for base16-emacs, if possible.

JamyGolden commented 3 months ago

Thanks for bringing this up and for the detailed information :pray:

Yes this should be fixed in Tinty, changing the base16-emacs repo isn't a desirable option (btw belak's repo is now https://github.com/tinted-theming/base16-emacs).

My initial thoughts of this problem is it would be a problem for theme files like base16-ocean.module.css or anything with more than 1 "extension" and this case emacs case is more of an edgecase compared to that situation. So my thoughts are going with a theme-file-extension property so someone could specify .module.css and we could specify -theme.el for emacs even though I do realise -theme.el isn't technically a file extension.

[[items]]
name = "base16-emacs"
path = "https://github.com/tinted-theming/base16-emacs"
themes-dir="build"
theme-file-extension = "-theme.el"
hook = "cp -f %f ~/.emacs.d/custom-theme.el && emacsclient -e '(load-theme 'custom t)'"

I think the name is more clear for most people (compared to file-override or theme-file-stem) and I like that we don't have to include a variable in the property value. What are your thoughts on this?

Having said that though, if you feel strongly against theme-file-extension, I'm happy with theme-file-stem (theme-file-stem = "%n" where %n is string variable of base16-ocean. It can be solved with "%n.module" in the base16-ocean.module.css scenario) as an alternative and I'll be happy to review the feature PR :coffee: