andreykaere / ixwindow

Iconized xwindow module for Polybar (for bspwm and i3wm)
43 stars 3 forks source link

Minor enhancements and crash fixes for i3wm #9

Closed anhnamtran closed 11 months ago

anhnamtran commented 1 year ago
andreykaere commented 1 year ago

Hi! Please, format your commit messages, according to this template. More to that, you should open PR to dev branch, because I do all the work in that branch and then merge dev to master, when it's stable enough.

I want ixwindow module to be the extension of xwindow, so for compatibility, I chose "Empty". If you want to have the option to put "Desktop" there, then you should make it as an option that can be specified in the config file.

I don't know how I can test the panicking on i3, because I don't have an external monitor, so I am gonna trust you on this one ... Because it shouldn't hurt anyway ...

anhnamtran commented 1 year ago

Sure thing. I didn't realize that you had a preference. It's probably helpful to have either a PR template or a little blurb for contributions as it'll probably reduce your workload a bit more.

I will make it so "Empty" is configurable.

For panicking on i3, for the ModeEvent, if you have a Resize binding mode (mine and on the default config it's Mod+R), that would cause the panic.

For the OutputEvent, I just did xrandr --output <MyOutput> --off but you would need 2 monitors for that.

anhnamtran commented 1 year ago

I see that you've addressed the empty XDG_CONFIG_HOME issue in dev as well. I will rebase onto that and see which commits are no longer needed.

As a side note, I see you're pretty actively working on this project. Would it be more convenient for you that I don't make PRs at all and just have a separate fork?

andreykaere commented 1 year ago

No, of course your PR is very helpful! I've been meaning to merge some of the critical fixes to the project when I have time anyway. But if you want some features I don't prefer, then you can either make them additional and I will be happy to merge them or just feel free to keep it in your own fork :)

andreykaere commented 1 year ago

About lacking CONTRIBUTION.md -- yes, I want to add this, but I can't find time to finally sit and rewrite a major part of code anyway. Besides, I've written quite a few bugs to fix. So, I will definitely do that in the future, just haven't gotten time yet.

Besides there are not so many PRs at the moment, so that's fine :)

anhnamtran commented 1 year ago

Okay great! I'll do some git magic to figure out which changes I have that you've potentially handled in the other branches, remove the commits, and reference those commits instead.

andreykaere commented 1 year ago

print_info_feature is the last branch that I was committing to, I believe. But I don't think there are a lot of duplications in your commits.

andreykaere commented 1 year ago

I think it's better to rebase onto dev. Maybe it's easier if you copy dev to your fork and rebase there and open new PR?

andreykaere commented 1 year ago

Why don't you want to rebase your changes onto dev? master is behind dev anyway and you will have to rebase (not merge!) onto devanyway for me to accept this PR

anhnamtran commented 1 year ago

Hi, I did rebase onto dev. Perhaps the confusion comes from the branch naming of my fork. The fork's master branch is now exclusively tracking the dev branch.

andreykaere commented 1 year ago

Oh, yes, I see it now. Okay, then just request my review once you think thid PR is ready to be merged and I'll take a closer look

andreykaere commented 11 months ago

I am going to close this PR due to inactivity. Some of the addressed problems were fixed in the recent merge (when implementing print_info feature), consider checking it out. Some of the suggestions I am going to implement manually, thank you for drawing my attention to them!

anhnamtran commented 11 months ago

Hey. Sorry for the inactivity and thank you for keeping an eye on this regardless, life got a bit hectic.

andreykaere commented 11 months ago

Hi! No problem! In fact, thank you for drawing my attention to some bugs and suggesting fixes!