mooz / xkeysnail

Yet another keyboard remapping tool for X environment
890 stars 112 forks source link

New features #133

Closed luizoti closed 1 year ago

rbreaves commented 2 years ago

So many edits.. why would anyone accept this PR? Maybe if you broke it up and actually said what each addition's purpose is. Not like I or anyone else other than mooz has the ability to merge PRs into this project as far as I know though.

luizoti commented 2 years ago

So many edits..

I hadn't noticed, amazing.

why would anyone accept this PR?

I don't know and at the moment I'm not really caring about it.

Maybe if you broke it up

I'm not in the mood, in fact, I uploaded this PR just in case someone wants to analyze it, no, if not, patience, my fork is working for me and that's fine.

actually said what each addition's purpose is.

Read the title of each commit, it's an extremely succinct description, but I remember that I tried to make it as clear as I could.

Not like I or anyone else other than mooz has the ability to merge PRs into this project as far as I know though.

I think the oldest PR in the repository is mine and it's been there for almost 1 years, the commits must be even older, I already know that mooz will most likely not follow through with the project, do what, xkeysnail is a good tool.

rbreaves commented 2 years ago

Well things have seemed stagnate for awhile, but I am sure some of that is that Mooz does not want to break what's working for most people. I have broken master before forcing him to kick a commit of mine out, but has kept others.

I was not trying to be rude, it just seems like a lot for a single PR and perhaps the individual comments are enough for Mooz. I do not know. Might be good if mooz could find co-maintainers for this project though.

We do need to get Wayland and other things fixed I am sure.

Also I hadn't realized you had helped me with a PR before and I need to test out your changes, thank you for that.

luizoti commented 2 years ago

Well, if he doesn't want to accept commits because he doesn't want to break main the project will die anyway, in that case it's better to continue with a fork.

About Wayland, I think it won't be that hard to get it working, but the problem is, is Wayland working? Are there methods that deliver what we need to detect the wm_name, wm_class? I remember doing research a year or so ago and I didn't find much. I use KDE and I don't remember where the Wayland-supported version is.

All this needs to be studied. I think some xkeysnail modules need to be rewritten, the use of global makes everything very complex and confusing, I started, but I myself don't use all the functions and I don't know when they apply to be able to modify them and with that I gave up.a

rbreaves commented 2 years ago

I think he's probably just not very active on github, but if you ping him over twitter he might get to it faster (as in measured in weeks or a couple of months still lol). What I did any ways, don't recall how I mentioned it exactly, but it probably helped that my PR's were relatively small.

And yea, Wayland is a moving target, but I did start a thread w/ some preliminary work in it. If someone doesn't address then I will try and circle back to it.

luizoti commented 2 years ago

Maybe I'll try calling him and see what happens, then I'll see that.

Your proposal was made on commig https://github.com/rbreaves/xkeysnail/commit/2a8dd0e54f53751b0daaf8370c4a9538942b48d2, right?

Maybe it's possible to implement with https://pypi.org/project/pywayland/ if the api has methods that deliver the wm_class and etc...

rbreaves commented 2 years ago

Yea, that commit was my proposal, but I think it may no longer be valid as I do not think that API is still be exposed in that way. I think an extension or some type of wrapper will need to be created now, which sucks as I use Budgie and cannot use Gnome extensions.. but I think there is still some method to make it work.

luizoti commented 2 years ago

Dude, wrap this command in a function, and put it inside the xkeysnail, after that, just call it inside the get_active_window_wm_class function.

gdbus call -e -d org.gnome.Shell -o /org/gnome/Shell -m org.gnome.Shell.Eval global.get_window_actors\(\)[`gdbus call -e -d org.gnome.Shell -o /org/gnome/Shell -m org.gnome.Shell.Eval global.get_window_actors\(\).findIndex\(a\=\>a.meta_window.has_focus\(\)===true\) | cut -d"'" -f 2`].get_meta_window\(\).get_wm_class\(\) | cut -d'"' -f 2 > /tmp/active_win

Hence, it looks for a way to detect if it is running on X11 or Wayland to decide when to run this function or the logic of X11.

If it works, it will be lighter and more practical without needing to sleep or read files.

luizoti commented 2 years ago

In the future, if an api for wayland appears that delivers everything that is needed, just implement the whole thing using it.

rbreaves commented 2 years ago

What I was trying to say though is that the active window is not supposed to be readable directly via "org.gnome.Shell" going forward, if I recall the devs are closing that off or already have to gnome extension access instead. So I think you'd need a helper app outside of that gdbus call.

luizoti commented 2 years ago

True, I had forgotten about this behavior of dbus not working as root, it really wouldn't be possible to integrate directly.

joshgoebel commented 2 years ago

@luizoti Did you ever speak with the current maintainer? I'm trying to figure out if this project is still actively maintained or not... looking at the last commit and state of PRs/issues it looks like the original author hasn't been involved in quite some time, or am I missing something?

luizoti commented 2 years ago

Diretamente não tentei ainda, ao menos não lembro ter tentado, para mim o dev original não deve voltar a tocar no projeto.

@luizoti Did you ever speak with the current maintainer? I'm trying to figure out if this project is still actively maintained or not... looking at the last commit and state of PRs/issues it looks like the original author hasn't been involved in quite some time, or am I missing something?

I haven't tried it directly yet, at least I don't remember trying, for me the original dev shouldn't touch the project again.

joshgoebel commented 2 years ago

I just reached out to the author via email and asked if they would be interested in passing on maintainer-ship of the project... if so, would you be interested in helping out?

luizoti commented 2 years ago

If he passes you, I can help when there's time, no problem.

joshgoebel commented 2 years ago

@luizoti You interested in a fork of the project if the author is simply non-responsive? I looked at the first 10 - 20 of your commits here and most of this stuff I would merge immediately without question (I even had some of the exact same thoughts as I reviewed the code). Some of the new feature stuff might require a bit more discussion. I'm guessing if I reviewed the entire list that trend would continue - ie most of this code would be mergeable as-is.

I don't need any more projects that I'm the sole maintainer of but if we got a small team together I think we could perhaps push this forward.

My first priority would be a test suite so we can make sure we avoid regressions moving forward in how the engine works. After there is a solid test suite in place then refactoring the core becomes a lot less scary.

luizoti commented 2 years ago

Some commits related to configurations, I would probably drop today, simply to reduce code, another I have to see what was done.

A few things I find interesting to look at in a refactoring:

  1. Removing the need for global variables, sometimes they make everything complex and confusing.
  2. Extend remap functionalities so that it is possible to remap each device according to wm_name, wm_class, if they are in full screen and if necessary to have different profiles for each situation.

These are ideas, possibilities.

joshgoebel commented 2 years ago

I see you adding version numbers in some of your commits, have you actually released your version anywhere in public where such things would matter?

luizoti commented 2 years ago

I just changed the version on github.

joshgoebel commented 2 years ago

Don't feel you have to change every little thing I point out... I'm probably going to start with 0.4.0 as-is, pull over as much of your work as seems reasonable and then build on top of that... A lot of times I'm just asking to understand what's going on - not expecting you to immediately go and make a fix.

Are you running xkeysnail in systemd but AS your own user? I'm a bit confused.

joshgoebel commented 2 years ago
+                if check_device.name == "py-evdev-uinput":
+                    # Exclude evdev device, we use for output emulation,

How did this get added, was this a problem you were actually having, you accidentally included the output device?

luizoti commented 2 years ago
+                if check_device.name == "py-evdev-uinput":
+                    # Exclude evdev device, we use for output emulation,

How did this get added, was this a problem you were actually having, you accidentally included the output device?

The evdev creates this device and needs to be ignored at certain times.

luizoti commented 2 years ago

Don't feel you have to change every little thing I point out... I'm probably going to start with 0.4.0 as-is, pull over as much of your work as seems reasonable and then build on top of that... A lot of times I'm just asking to understand what's going on - not expecting you to immediately go and make a fix.

Are you running xkeysnail in systemd but AS your own user? I'm a bit confused.

As root, systemd was configured to give the user dbus access, to generate desktop notifications.