Customisable config file storage paths goal on future enhancements

[Pranav-Badrinathan]

Hello! I was just looking around for a clipboard manager on Wayland today, and stumbled upon this absolute gem of a project.

I really like clipse, especially with it being a TUI app, and really wanted to help out by contributing something.

Looking at your list of future enhancements, I came upon the "Customisable config file storage paths" goal, and wanted to ask what you meant by it... Do you mean having a config file that is not stored in the default ~/.config/clipse/? If so, how do you envision knowing where the config file is to read from it? The only ways I can think of is via cli arguments or setting environment variables.

If you permit, I would like to try my hand at adding a config file for item limits, keybinds, etc. and making it's storage path customisable.

[savedra1]

Hi @Pranav-Badrinathan 👋

Thank you so much for the kind words and for planning to contribute!

To clarify what I meant, I was thinking to keep the .config/clipse dir and add a new config file that could potentially allow the specification of custom paths for the other files. I would say the custom paths is less important than the custom binds/limit size etc.

That was just initial thoughts though so feel free to experiment with any other solutions.

Also thought about combining the custom config with the theme config file. It would make that file more complex but probably save some lines of code from having to manage a new source file.

Overall though I'd want to prioritise the user experience so that's the best thing to keep in mind. Adding addition cli args for updating the config on the fly may be a good way to manage this.

Excited to see how you get on, let me know if I can assist/clarify further :)

[Pranav-Badrinathan]

Thanks for the prompt reply!

To clarify what I meant, I was thinking to keep the .config/clipse dir and add a new config file that could potentially allow the specification of custom paths for the other files.

So something like this (assuming .json format for config files, as you are currently using that, but it will be extremely easy to use .yaml, .toml, etc. if you want):

{
  "sources": [
    {
      "type": "theme",
      "path": "~/path/to/theme/file/here",
    },
    {
      "type": "config",
      "path": "~/path/to/config/file/here",
    },
    {
      "type": "dir",
      "path": "~/path/to/entire/dir/to/include/here",
    }
  ]
"other config options like item limits, key rebinds, etc. here"
}

Also thought about combining the custom config with the theme config file. It would make that file more complex but probably save some lines of code from having to manage a new source file.

Personally, I think keeping them separate is nice, due to separation of concerns. Having multiple themes you switch between can also be a feature in the future. And if we go with the sourcing of files as above, it is essentially managing any number of source files 😅.

Adding addition cli args for updating the config on the fly may be a good way to manage this.

Auto updating on config save could also be something good then, for UX. Makes configuration (especially theming) much easier.

I implement the system above and test it out, and report my findings. Do tell me if something is missing/not preferrable in my proposition.

[Pranav-Badrinathan]

As for how the configuration data split between multiple files (the type "config" files specifically) could be handled nicely, it could be represented as a static struct that holds the config state and is loaded upon calling clipse -listen.

For duplicate tags (like referenced file sets item-limit to 50, but current file sets it to 100), the last instance of the tag is used, with the first overwritten.

[savedra1]

Hi @Pranav-Badrinathan,

Yeah that example looks great! I know toml/yaml is more commonly used for these configs recently but I'd prefer sticking to json for now to maintain consistency like you suggested.

Also I agree the separate file system works better here to allow modularity for future developments, great point.

I like the idea of update on save. Currently the theme file is loaded each time the TUI view is ran which (given how the TUI only tends to be open temporarily) achieves the same effect. It sounds like what you're suggesting here, including the load from clipse -listen would be an optimisation of this as would potentially require less file processing, but if this method still requires a check of the config each time the TUI is loaded though there may not be a need for the update on save.

Your suggestion sounds good to me and I'm excited to see your findings :smile:

[Pranav-Badrinathan]

Great! 😄

Ideally, it would not re-source the config each time the TUI opens, but this is probably lightweight enough to not see much of a difference.

I unfortunately am busy today, but I will start on this tomorrow!

[Pranav-Badrinathan]

So I was working on the code and noticed you flip between using "historyFile" and "configFile" to refer to the clipboard_history.json file. I am probably going to rename this for consistency and use historyFile to refer to the actual clipboard history, using configFile for the new config file.

Also, the file clipboardConfig.go only really has logic pertaining to managing the clipboard history, not the config. Would it be okay to rename this to reflect this, and split the actual config file (that I am working on) into a different file?

I apologize if I am asking too many questions here, if you would rather prefer to address any of my choices in the final PR, let me know and I'll do what I think would be best for now, and we can discuss on the final PR.

[savedra1]

Hi @Pranav-Badrinathan ,

I appreciate your attention to detail here and I agree with your approach towards naming conventions. I did think the clipboardConfig.go file was getting a bit busy.

Please feel free to make these adjustments and I can make suggestions when the PR is raised (if necessary).

One extra thing to add in case you are not already aware, since you picked this up there has been a new commit for implementing two new keybindings, one for pinning items and one for toggling the pinned view. See PR#27 for more info. You may want to consider this for the custom binds you add.

Thank you again and I look forward to seeing your progress 😁

[Pranav-Badrinathan]

Hey @savedra1, there is something I wanted your input on. So taking in an arbitary number of source files (ones that can be config files themselves, and thus define more source files and settings) means that multiple config files could define the same setting. To address this, I decided we could just pick the last set value and use that, allowing the end user to overwrite the same value in different files. If you take a look at config.go, you can see how I am loading the source files.

The problem arises because Go's primitives are not nilable, and it does not have an optional type. So there is effectively no way to say "This value is undefined", as the zero values themselves can be values set in the config. Currently, I am checking to see if the field of a source file is a zero value (meaning it is not defined in json), and if no, use it to overwrite the old value. This means a zero value, like false, can never overwrite a non zero value.

The only way I see to fix this behaviour as of right now is to have the config struct's members be pointers to primitives, so they are nilable and can be checked to see if the value is undefined. Any thoughts or suggestions?

relevant commit.

EDIT: I might actually have a fix for this... I'll need to test it tomorrow, but this might be an easy fix.

[savedra1]

Hey @Pranav-Badrinathan, it looks like you've made some great progress!

Honestly I can't think of any immediate fixes myself, but if enforcing a single 'master' config file that can only define paths for other 'non-master' sources (like theme/history/imgs etc) would solve the issue, I would be happy with that solution. EG if the base dir .config/clipse has to be used for at least clipse_config.json, but then other source locations can be specified within that file.

I will have a deeper look into this though when I get the chance, I see you may already have a solution as well so will keep an eye out for that :smile:

[savedra1]

Hi @Pranav-Badrinathan,

After some more thought I do think a single config file would be easiest here, maintaining the need for the original .config/clipse dir for at least that file - if you have a nice solution to the above though do let me know :smile:

[Pranav-Badrinathan]

Oh, sorry for not replying, but yeah I managed to fix it. I didn't want to complicate the use of the config in code, so I didn't want to store it as a struct of pointers (nilable). However, I realized that json.Unmarshal internally already does what I wanted. It internally only overwrites the information that is specified in the json. So I just rearranged the code a bit and it works as intended now! I just need to check that I have updated references, and am importing theme data properly, then I'll open a pull request.

Further improvements like allowing multiple themes to perhaps be selectable via ui or hotkeys or something, or adding keybinds to the config (it looks dreadfully bleak at the moment, with only a list of other sources, a max copy limit, and a configurable location for the history file), etc. can be done in other subsequent PRs.

EDIT: Another convenience feature could be specifying window height and width in the config. Each time I boot up my computer, clipse resets back to a fullscreen view.

[savedra1]

Oh wow @Pranav-Badrinathan great spot!

The items you mentioned that would currently exist in the config sound perfect for the first iteration!

The window resizing would also be great, I think this would be highly dependant on the user's system though which could be tricky, which is why the advice for that is currently to let the window manager do the heavy lifting there.

I look forward to receiving the PR! Once we get that merged I'll be ready to ship out the next release :heart_eyes:

[Pranav-Badrinathan]

Just finished reading all the theme files from config and using them for the themes. So that is now configurable. Last 2 things I need to do before PR is adding support for relative file paths ("~/" and maybe "./") in the config files, as it currently requires an absolute path, and last thing would be to use os/fsnotify and use syscalls to only re-load the config and all related source files upon change.

Even if you do not want to support the file reload on change in this PR, I think at least relative file paths are a must, so I'll first implement that.

[savedra1]

Amazing! thank you for keeping me updated. I look forwarded to testing out your PR when it comes through and let me know if there's any way I can assist :smile:

[Pranav-Badrinathan]

DONE! I think at least. I will update the README and then open a Pull Request.

[savedra1]

Thank you so much @Pranav-Badrinathan! I will test as soon as I'm back from vacation and update the PR 🤩

[Pranav-Badrinathan]

Thank you so much @Pranav-Badrinathan! I will test as soon as I'm back from vacation and update the PR 🤩

Take your time and have fun on vacation :hugs:.

[savedra1]

36 merged!

Massive shout outs to you in the next release notes. Thank you so much for the excellent work :rocket: