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

Feature: File-extension #32

Merged
merged 11 commits into from
Jul 1, 2024

Conversation

1ynnshell
Copy link
Contributor

@1ynnshell 1ynnshell commented Jun 28, 2024

There still needs to be tests written and the documentation to be updated, but I wanted to get this out there to get some opinion on it first before committing to those.

First, the requirements for emacs that we are trying to meet:

  • Theme file must end in -theme.el
  • Theme file must be within the users custom-theme-directory (this one is easy)
  • Theme file must be the same theme name as the theme provided within
  • Theme must be loaded by theme name via emacsclient

hurdle one was just properly identifying the emacs scheme files. The addition of an Optional parameter, theme-file-extension works around that: if this parameter is enabled it will search for the scheme-name with the theme-file-extension together.

For example, base16-nord normally will look for any file that matches base16-nord.*. With theme-file-extension="-theme.el", base16-nord will now match for any file that is base16-nord-theme.el. This solves our first hurdle.

The second hurdle is easily handled by hooks, but the third and fourth require an additional macro within the hook: %n, which provides the full scheme name.

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

This solves all the issues involved with tinty and supplying an appropriate theme, via tinty apply, to the emacs daemon. There are some other issues, but those are related to my implementation of the hook (will most likely need to be moved into a script for more elegant handling of old -theme.el files, persistent application of the theme, etc.)

Let me know what you think of the solution and please provide feedback. The goal is to get Tinty working with emacs the Tinty way :)

(Related to task #30)

@1ynnshell 1ynnshell requested a review from a team as a code owner June 28, 2024 06:50
@1ynnshell 1ynnshell requested review from belak and removed request for a team June 28, 2024 06:50
@1ynnshell 1ynnshell marked this pull request as draft June 28, 2024 07:04
@1ynnshell
Copy link
Contributor Author

Found another case for the theme-file-extension to give us a more broad look at what requirements we need to meet.

[[items]]
name = "base16-qutebrowser"
path = "https://github.com/tinted-theming/base16-qutebrowser"
themes-dir = "themes/default"
theme-file-extension = ".config.py"
hook = "cp -f %f ~/.config/qutebrowser/theme.py && pgrep qutebrowser && qutebrowser :config-source"

@JamyGolden
Copy link
Member

I just saw this pr manually, it looks like belak was tagged automatically so I didn't get a notification. I'll go through this tomorrow! (or later today I should say)

Copy link
Member

@JamyGolden JamyGolden left a comment

Choose a reason for hiding this comment

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

Looks good to me! I tested it out locally and is working as expected. Can you add a test for this? I've created one, but if you want to do it differently and do a different test that's ok too!

tests/cli_apply_subcommand_tests.rs:

#[test]
fn test_cli_apply_subcommand_with_config_theme_file_extension() -> Result<()> {
    // -------
    // Arrange
    // -------
    let scheme_name = "base16-uwunicorn";
    let (config_path, data_path, command_vec, cleanup) = setup(
        "test_cli_apply_subcommand_with_custom_schemes",
        format!("apply {}", &scheme_name).as_str(),
    )?;
    let config_content = r#"
[[items]]
path = "https://github.com/tinted-theming/tinted-alacritty"
name = "tinted-alacritty"
themes-dir = "colors"
hook = "echo \"expected alacritty output: %n\""

[[items]]
name = "base16-emacs"
path = "https://github.com/tinted-theming/base16-emacs"
theme-file-extension="-theme.el"
themes-dir="build"
hook = "echo \"expected emacs output: %n\""
"#;
    let expected_output =
        "expected alacritty output: base16-uwunicorn\nexpected emacs output: base16-uwunicorn\n";
    write_to_file(&config_path, config_content)?;

    // ---
    // Act
    // ---
    utils::run_install_command(&config_path, &data_path)?;
    let (stdout, stderr) = utils::run_command(command_vec).unwrap();

    // ------
    // Assert
    // ------
    assert_eq!(stdout, expected_output,);
    assert!(
        stderr.is_empty(),
        "stderr does not contain the expected output"
    );

    cleanup()?;
    Ok(())
}

@1ynnshell
Copy link
Contributor Author

@JamyGolden Thank you for writing that test, I would be happy to write them in the future but I appreciate it. Added the test in, passes just fine.

@1ynnshell 1ynnshell marked this pull request as ready for review July 1, 2024 19:24
@JamyGolden JamyGolden merged commit ebdb1a5 into tinted-theming:main Jul 1, 2024
5 checks passed
@JamyGolden
Copy link
Member

I've published 0.16.0 with the change, thanks for spotting the problem and doing the work!

@1ynnshell 1ynnshell deleted the ll/feature-file-extension branch July 2, 2024 06:03
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