search

tmerr / i3ipc-rs

A Rust library for controlling i3-wm through its IPC interface
MIT License
106 stars 33 forks source link

Add support for container marks #51

Closed Emantor closed 2 years ago

Emantor commented 4 years ago

Container marks have been available in i3 since at least 4.12, so they do not require a feature-gate.

Signed-off-by: Rouven Czerwinski rouven@czerwinskis.de

sophacles commented 4 years ago

Hi @Emantor - I have a PR open here: https://github.com/tmerr/i3ipc-rs/pull/45 that does this also. Our respective PRs differ in that marks in yours is Option<Vec<String>> and mine is Vec<String>. Ultimately I don't really have a strong opinion on which gets accepted, but I do have a question for you:

I'm still early in my rust journey and I'm trying to understand why to choose one over the other. I chose the bare Vec to match nodes and floating etc, but I can see some value also in having the Option. It would be even better if the ? operator worked on struct member access though. If you have time to help a n00b out, mind expanding on your thoughts here to help my rust skills?

Emantor commented 4 years ago

IMO it depends if you want a one to one mapping or a mapping which is more rust compatible (using types only available in rust). Mapping the empty list to options saves me a recursion over the entries and lets me short-circuit inside the code. I find the option just nicer to work with :grin:

sophacles commented 4 years ago

Thanks!