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

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

Closed
1ynnshell opened this issue Jun 27, 2024 · 1 comment
Closed

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

1ynnshell opened this issue Jun 27, 2024 · 1 comment
Assignees

Comments

@1ynnshell
Copy link
Contributor

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
Copy link
Member

JamyGolden commented Jun 27, 2024

Thanks for bringing this up and for the detailed information 🙏

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 ☕

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

No branches or pull requests

2 participants