C-Loftus / talon-ai-tools

Query LLMs and AI tools with voice commands
http://colton.place/talon-ai-tools/
MIT License
46 stars 17 forks source link

Renamed the settings file to allow users to override it #19

Closed jaresty closed 6 months ago

jaresty commented 7 months ago

I prefer for configuration to not be committed to the repository since it makes it difficult for users to override the configuration. This change allows users to see an example that they can easily copy and paste into their own configuration folder, which will hopefully make it easier for people to use this repository without having to make their own fork.

C-Loftus commented 7 months ago

I think this can be done easier by matching on the hostname. It is easy to override this so I personally prefer having this here as a working file rather than having a dummy file. I think I'm gonna keep it as is for now

jaresty commented 7 months ago

What do you mean by "matching on the hostname"? It's not intuitive to me that configuration settings would be baked into a repository. I think this will encourage forking of this repository, which I think is bad for contribution.

jaresty commented 7 months ago

FWIW - I'm not opposed to having a setup script or something that copies the configuration example file to a real location in this repository that is in the .gitignore file - I've seen that work fine in other open source repositories before.

C-Loftus commented 7 months ago

You can put the hostname in the context header and it will override any settings without the context header. So the config is decoupled and easy to override. I am not sure I will change this since knausj also has settings in done in a similar way

I am a bit unsure on this so going to leave it open for time being.

jaresty commented 7 months ago

I'm not a fan of this approach, but at the very least, can you put a commented out example of how to override it? I still don't know how to do this myself. If we follow the configuration practice of community, we will likely end up with the same results (i.e. many forked repositories that do not upstream contributions) over time.

jaresty commented 6 months ago

I'm raising this issue on the community repo on the same topic. Thought I'd share here for visibility too: https://github.com/talonhub/community/issues/1384

jaresty commented 6 months ago

Regarding using the host name to override: I do commit my personal configuration to a repository and I also deploy it to multiple machines, so relying on the host name isn't convenient since it varies per computer which means I'll need a custom deployment script just for that. I haven't had a need to override any settings in community yet that were hard coded into the configuration file. Many of those have defaults that aren't committed so you can override it with a settings file anywhere in your user folder. There are a few settings that are hard coded, though, and I have struggled with how to change them since I doing want to fork the repository.

C-Loftus commented 6 months ago

OK here is what I'll say. I think having a '.example' file might not be immediately intuitive for new users. And I like having a real live talon settings file where you can edit, since the vast majority of people do not fork and the ones who do haven't really complained. But that being said I do get where you are coming from. I think there are many scenarios where voice commands will need forks given the particularities of user set ups and or accents, but this is not one of them.

I am willing to merge this if we

jaresty commented 6 months ago

These all sound good to me. I'll put that together soon.

jaresty commented 6 months ago

I tested without the settings file and it does work as expected.

C-Loftus commented 6 months ago

Thanks, merged