yazgoo / umberwm

:ram: a minimalistic X window manager based on tinywm, inspired by qtile.
57 stars 3 forks source link

Move main.rs into this repo #19

Closed mfdorst closed 3 years ago

mfdorst commented 3 years ago

I think I get why you separated main.rs into its own repo - it kinda makes sense if you expect people to leave the main crate untouched and only modify things in the config (main.rs), but I don't think that's a likely use case. Certainly for me I want to treat this repo basically like it's DWM and customize it myself. Given that use case, having this crate as a cargo dependency of the actual binary doesn't really work, since the Cargo.toml of myumberwm just references this repo, not my personal one.

Furthermore, adding a main.rs to this crate actually wouldn't damage that use case. You could still pull this crate in as a dependency in myumberwm and only the library parts would get compiled.

That change would also enable my installer script to work (see #18). I'm not sure if that's something you want as part of this repo, but I think it improves both utility and usability quite a bit.

Is this a change you are open to?

yazgoo commented 3 years ago

Thanks for your contribution.

I'd rather not have a main.rs in the repo to make sure people have their custom one and do not use a default which will surely not be adapted to their use (for example my usage is based on my keyboard layout - bépo - which most people won't use)

If I understand your use case correctly, you want to be able to modify umberwm crate without publishing and compile main.rs with it ? I also sometime need that (when I'm developing) and there are several other ways to do that. The one I'm using is to have src/main.rs be a symlink to myumberwm/src/main.rs

Then, in my xinitrc, I look for the most recent binary between umberwm and myumberwm via

umberwm_path=$(/usr/bin/ls -t /home/yazgoo/dev/*umberwm/target/debug/*umberwm | head -1)

That way I'm always sure to be running umberwm or myumberwm based on which one is the more up to date

mfdorst commented 3 years ago

Well, we could work on providing a more generic default configuration file, the way that other source based window managers do. But personally I found that I didn't require that many edits to make your config usable to me.

The other major benefit it would provide is that I'm planning to make some improvements to the library parts, for instance I'd like to be able to assign keybindings like Mod+Shift+Q that are different from Mod+Q. That requires me to change how keybindings are configured, and if main.rs is maintained separately from the main repo, that becomes a logistical nightmare, because my changes would have to be made in two different repos, and then you end up with incompatible versions of umberwm vs myumberwm which sounds like more trouble than it's worth.

mfdorst commented 3 years ago

I'm also considering the idea of providing a non-code configuration file as an option. Something like a toml or yaml file that would get read by main.rs, but with the option of turning that off and opting for Rust only configuration, or even a hybrid approach where maybe you have some custom actions you want to write code for, but everything else can be configured in toml. If that sounds like a good idea to you, I think it would make much more sense as part of the main repo rather than being separate.

Also, anyone who doesn't want to make edits to lib.rs need only add umberwm as a cargo dependency for their own repo, the way you have for myumberwm. Adding a main.rs here does not remove that functionality.

yazgoo commented 3 years ago

you make a compelling argument :) Let's add main.rs here then. As for the configuration file, it depends on how much complexity this adds to the WM. I'd like to keep it simple

mfdorst commented 3 years ago

My vision for the config file is pretty simple. It would basically amount to adding #[derive(Deserialize)] (from serde) and a couple lines in main.rs to search for config files and read them.

mfdorst commented 3 years ago

Of course, that would require adding serde as a dependency, which would increase initial compile times.

yazgoo commented 3 years ago

I've already added miniserde as a dependency, maybe we could use that

mfdorst commented 3 years ago

@yazgoo I looked into it. miniserde only supports JSON (by design). I would prefer not to use JSON as the configuration language.