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

feat: select subtemplate based on the scheme used #66

Merged

Conversation

dive-deeper
Copy link

@dive-deeper dive-deeper commented Oct 28, 2022

Solves #65. It is not ideal, since I call find_template twice, but it gets the job done. I could change the find_template function to also receive the scheme as a parameter and give that back if a subtemplate with the scheme name is found, but well I'd like to first hear from you :)

@Misterio77
Copy link
Owner

Misterio77 commented Oct 29, 2022

Hello! Thanks a lot for the contribution :D

It's a very valid usecase.

I was wondering if perhaps we could make this more explicit and transparent instead of it being implicit behaviour (a subtemplate/scheme name collision is unlikely, but very possible).

What if the user could use a {scheme} placeholder in their flavours configuration to refer to the scheme slug? That would make it work with any of the fields. For example, for your usecase (using scheme as subtemplate):

[[items]]
file = "~/.config/sway/config"
template = "sway"
subtemplate = "{scheme}"
hook = "swaymsg reload"

Would you be interested in making this change?

@Misterio77
Copy link
Owner

Misterio77 commented Oct 29, 2022

The shell option does something similar with its {} placeholder, to support a variety of shells with different invocation syntaxes:

let full_command = shell.replace("{}", &command);

@dive-deeper
Copy link
Author

Hey sorry, I was on holidays and just now came back. Thanks for your feedback! Sounds like a good idea with the {sheme} placeholder to make it more explicit. I'll implement it over the weekend :) I'd keep the behaviour that if no subtemplate with the scheme name/slug is found, then "default" is selected.

@Misterio77
Copy link
Owner

Welcome back! Hope you rested well.

I'll implement it over the weekend

Thanks a lot!

@dive-deeper
Copy link
Author

dive-deeper commented Dec 11, 2022

Didn't get to it last weekend, but have implemented it now. The search for a scheme dependent subtemplate now only happens if the subtemplate's value in the config is set to {scheme}. If no subtemplate with the name of the scheme is found, default is selected :)
I tried to document the new option/behaviour. It's a little verbose, but couldn't describe it in less words...

@Misterio77 Misterio77 self-requested a review April 6, 2023 16:54
@Misterio77
Copy link
Owner

Looks good to me! Thanks a lot for the contribution (and sorry for the wait)!

@Misterio77 Misterio77 merged commit 26842e8 into Misterio77:master May 2, 2023
Misterio77 added a commit that referenced this pull request May 2, 2023
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