dracula / tmux

🧛🏻‍♂️ Dark theme for tmux
https://draculatheme.com/tmux
MIT License
660 stars 312 forks source link

Feature/theming #296

Closed Theoreticallyhugo closed 1 month ago

Theoreticallyhugo commented 1 month ago

the idea here is to allow fully custom colours, both in the hex-code, as well as the name and number of available colours, whilst keeping changes minimal.

i created this in an attempt to slowly but surely recover and go towards the goal of #224.

changes are documented on the documentation branch im working on, including some info to use other themes with dracula

what changed: the colour variables now live in a separate file, which is sourced per default. a new colours.sh file can be created where the user wants ( for example ~/.config/tmux/colours.sh) and then sourced with the option set -g @dracula-colors "$current_dir/colors.sh"

so, for default users theres very little changes under the hood and none that they could observe. however this little changes opens up many possibilities for customisation.

ethancedwards8 commented 1 month ago

I'm going to close this in favor of #224

I would appreciate any feedback you had on that PR.

Theoreticallyhugo commented 1 month ago

I'll look into it. however I'm somewhat saddened with the lack of discussion of pros and cons between #294 and #296 😅

ethancedwards8 commented 1 month ago

It appears to be more flexible. I don't like the idea of users having to edit the programs script files. Modifying the files directly often inhibits updating the plugin, due to git detecting changes. Editing the .tmux.conf file removes this issue.

Theoreticallyhugo commented 1 month ago

but thats an issue i kept in mind and actually the reason i created this solution in the first place. this would allow a user to create a file with whatever variables in any location on their machine. so, if a user had their tmux.conf under vcs, they could put the colours.sh file in the same dir and point the tmux option to that file. basically the idea is to have one tmux option to set all colours at once, without having to touch the programs code at all. this could also be achieved by a long string appended to the tmux option, but would be quite ugly to read. hence the external file solution. basically #294 s solution is to have a separate tmux option for each and every colour, whilst i wanted to reduce that to exactly one.

ethancedwards8 commented 1 month ago

That is a very good point. I did not consider having the file outside of the plugin directory. I just assumed that it would be the one to be edited.

I think I would be more open to this idea if instead of having a file in the directory, which can be misleading, we leave the default colors in dracula.sh but then overwrite them if the script detects the presence of a configuration file. That way, users are not tempted to edit the colors.sh file in the plugin repo, causing issues with updating. We could also point out that putting the custom color file in VCS could be beneficial to some users.

We would also need to provide an example configuration (essentially what you currently have in colors.sh right now) in INSTALLATION.md. I would not merge this without it.

Also, could you think of a solution where users are able to place colors into .tmux.conf instead of having to create an entirely new file just for colors?

Theoreticallyhugo commented 1 month ago

i like the idea with keeping the default colours and just overriding them. i realised earlier today that not doing it that way may lead to weird issues and crashes when a user forgets assigning a variable that is used somewhere 😅

im gonna look into solutions, where the info could be placed inside the tmux.conf.

besides ive been working on configuration explanations/ installation instructions/ templates for themes in my documentation branch. i know a few people who would love to use this plugin if they could use it with guvbox or catppuccin and so im working on default configs that can be copied.

i can make sure to put instructions into installation.md for now, but im also assuming that it may move when the bigger documentation is getting ready to get merged.

i'm very happy that this solution may get merged. i've been hoping for this day for the past two years 😅

Theoreticallyhugo commented 1 month ago

converting this to a draft, for this is currently work in progress. i've added info to the readme and install.md, pointing to the instructions in the docs dir. i'm currently testing the catppuccin values, and have asked a friend to check gruvbox, as he knows that better than me. both of those plug and play configs are configured without #224 ofc. we could consider implementing #224 in this pr, or wait with this pr till the #224 successor is merged, in order to finalise documentation on color theming. for that id like to hear your thoughts @ethancedwards8.

im still looking for a solution that would allow us to configure everything in tmux.conf.

ethancedwards8 commented 1 month ago

converting this to a draft, for this is currently work in progress. i've added info to the readme and install.md, pointing to the instructions in the docs dir. i'm currently testing the catppuccin values, and have asked a friend to check gruvbox, as he knows that better than me. both of those plug and play configs are configured without #224 ofc. we could consider implementing #224 in this pr, or wait with this pr till the #224 successor is merged, in order to finalise documentation on color theming. for that id like to hear your thoughts @ethancedwards8.

im still looking for a solution that would allow us to configure everything in tmux.conf.

Thanks for adding documentation.

Generally, it is advisable to do one thing at a time in FOSS projects. It keeps Git history clean, and makes it easier on maintainers and users to digest what is happening. For that reason, I think we should keep #224 and this PR strictly separate.

I'm not sure that the individual theming of each widget (git, kubernetes, terraform, etc) is necessary? Doesn't just setting the global colors change the widget colors as well?

Theoreticallyhugo commented 1 month ago

i saw that you commented and will look into fixing later.

the individual theming of each widget is a feature that is not added by this pr and which i still consider important. there are cases in which multiple widgets use the same color by default, which is kinda ugly/ weird when they're next to each other. take for example all network widgets, which come in the same colors. global theming would change the color of each widget in the same way, which wouldn't solve the issue of indistinguishability. besides, it is a very nice feature for a color theme and we do already have it implemented.

Theoreticallyhugo commented 1 month ago

@ethancedwards8 i fixed the comments you had and changed the code to get rid of the external file. i'm still working on documentation for the plug n play themes (only catppuccin and gruvbox so far), but the theming option itself should be fully functional now. will do some testing

ethancedwards8 commented 1 month ago

Everything here looks really good. I've thought about everything for a few days, and I can't really say that there is anything I would change. I am open to hearing feedback from other contributors.

Theoreticallyhugo commented 1 month ago

awesome! ill add more prebuilt theme stuff when we have merged a pr that allows for more customisation :D