enigo icon indicating copy to clipboard operation
enigo copied to clipboard

How to revive the crate?

Open pentamassiv opened this issue 2 years ago • 25 comments

Describe your Question How can we revive this crate again?

Describe your Goal The project has been dormant for a few months and PRs have accumulated. The download numbers on crates.io show that it is still actively being used and it is worth updating. I am also using it for a personal project and would like to see it thrive. I offered @pythoneer that I can help out and there have been other people on the discord channel who would help too. Let's discuss how we can proceed from here.

TODOs

  • [x] Add co-maintainers/reviewers
  • [x] Add Github Workflows to ensure the quality of the code and make reviews easier
  • [ ] Enable branch protection to ensure only building (and hopefully working) code gets committed to the main branch
  • [x] Review pending PRs
  • [ ] Check Rustdesks version of enigo for differences and work on merging them so they can use the crates.io version
  • [x] Go through all issues and check if they are still relevant
  • [x] Work on reducing the number of issues
  • [x] Publish a new version of the crate

Open questions:

  1. @pythoneer would you like to continue maintaining the crate if you get some help or are you looking for somebody to take over?
  2. Who else is willing to help maintain the crate?
  3. How can @pythoneer and others build trust in the new co-maintainers?

pentamassiv avatar Jan 15 '23 22:01 pentamassiv

I am willing to help maintain the crate and have enough time to give it an update. However I only have Linux.

pentamassiv avatar Jan 15 '23 22:01 pentamassiv

@pentamassiv thank you very much for the time and effort you put in! I agree with the working items you already put in place. Not sure if we need to takle all of them at once in order to release a new version but its a great guideline to get things going.

I have added you as a maintainer – if others want to be part of the team, please respond either to this issue or in the discord channel (or by any other means you see fit)

One thing i would like to add is to find a way to better test the library. This was a very, very manual process and it was one of the mayor time sinks in the past. There is barely any surface for unit-tests and i am still not sure how to even come up with a way to make integration tests as it almost always require a graphical system running. Unfortunately even running Windows in a VM (for local tests) is not a very good fit as VM's (for whatever reason) can behave quite differently than an actual running system.

pythoneer avatar Jan 15 '23 23:01 pythoneer

I agree, testing is difficult. I have created a few Github Workflows to at least check if everything builds. Will open a PR tomorrow

pentamassiv avatar Jan 15 '23 23:01 pentamassiv

How should we proceed with the PRs? I believe you have granted me the permission to merge PRs but so far I have not tried it out. How do you imagine the process of getting PRs merged to look like?

Should I open PRs and you have a final look at them and merge them or should I merge what I think is good?

pentamassiv avatar Jan 18 '23 21:01 pentamassiv

I would say just go ahead if you feel confident about the changes – especially new stuff like the github flows because i am not very used to it either. I trust your gut feelings :)

pythoneer avatar Jan 22 '23 16:01 pythoneer

I emailed a while back that I was willing to help with maintaining this crate and have confirmed that I am still willing to help out.

BrettMayson avatar Jan 26 '23 09:01 BrettMayson

Great, I see you already received the permissions to merge PRs. Do you have a Windows and Mac machine? I only use Linux so it would be great if you were able to test the code on the other platforms.

pentamassiv avatar Jan 27 '23 20:01 pentamassiv

I have access to all 3. I'm working on automated tests for all 3 using a Firefox browser as a cross-platform way to receive events.

https://github.com/BrettMayson/enigo/tree/tests

The test is working on Linux, and hangs on Mac and Windows, and not sure why yet. Hard to debug when I don't have access to the machine. The tests run fine locally on Windows and Mac.

BrettMayson avatar Jan 28 '23 01:01 BrettMayson

@BrettMayson that is a very nice idea – i wasn't sure if such an approach could even work because i am not sure how these headless browsers even work and if there is a "dummy" display server running in those OS instances. All i noticed back then was that there is a difference in running enigo on a virtualized instance of windows or native. Because i wanted to have "just" VM i can fire up under linux but ended up with an old laptop to manually test for windows. Not sure if this is related.

pythoneer avatar Jan 29 '23 15:01 pythoneer

Very cool, I used your approach and tried to fix it but so far, I did not have any luck. It works under Linux but not the other platforms :/

I replaced the dependency "websocket" with "tungstenite" since "websocket" is deprecated and they suggested using "tungstenite". Here is my code: https://github.com/pentamassiv/enigo/tree/tests

pentamassiv avatar Feb 04 '23 23:02 pentamassiv

I can't get it to work on the other platforms. You said the test works if you run it on your local machine so how about we try the following:

Local testing

  • Use the browser tests

Github Workflows

  • Use the browser tests only on Linux
  • Try to get something like this keylogger to work ONLY for remote tests on all platforms

pentamassiv avatar Feb 10 '23 01:02 pentamassiv

Out of town at the moment, I do plan on trying further to get the browser test running on Mac and Windows

BrettMayson avatar Feb 10 '23 01:02 BrettMayson

@pythoneer I removed Travis and Appveyor from the repo since they are not used. Are there any permissions that were granted to them that you can also remove in the settings? I don't have access to them so I can't check.

pentamassiv avatar Feb 26 '23 08:02 pentamassiv

@pythoneer , I think we are ready to release a new version of the crate. What do you think? There are only two more unmerged PRs:

  1. Cleaning up the code for macOS
  2. Allowing enigo to work if the $DISPLAY env variable is not set

The first PR isn't very important and can be merged and released without a bump in the major version. I don't think the second PR is ready to be merged. I was thinking about changing the API a little bit so that we can select settings when a new enigo instance is created, but I haven't thought it through so it will take a little bit of time.

There are a number of important fixes though and I will be traveling soon so I believe we should release a new version within the next few days.

pentamassiv avatar Mar 14 '23 01:03 pentamassiv

That sounds like a good Idea. Do we currently have (big) API changes? I would like to take some time (later after the release) and thinking about removing some of the panics!() that are currently in there or provide like try_* versions that would not break the api – so we don't have to change the api in small increments but in one big swoop.

pythoneer avatar Mar 15 '23 15:03 pythoneer

The only API change in this version is:

  • Traits: Added the functions main_display_size and mouse_location to MouseControllable

There are some other changes but they change the behavior. I bumped up the version number so I think the crate is ready to be published. Feel free to run cargo publish if you agree.

Yes, removing the panics is a good idea. I started work on that but then stopped to first get the other PRs merged. Since this is a library, I think we should never panic. We should always return results or options so that the person using the library can decide on what to do with a failure.

pentamassiv avatar Mar 25 '23 15:03 pentamassiv

Thanks for your hard work @pentamassiv ! https://crates.io/crates/enigo/0.0.15

pythoneer avatar Mar 25 '23 21:03 pythoneer

Hey Guys, Good work. I could help maintaining as well. If I may add to the work-list.

  1. kick-out xdo dependenciy, instead use xlib for X11, and later on worry about wayland seperatly. (I do not have a PR yet but it could be added to the wait-list).
  2. I can perepare a PR for key aliasing say for instance arrow keys, UpArrow, could be ArrowUp.

uwejan avatar Mar 26 '23 00:03 uwejan

Feel free to open PRs :)

Regarding the two suggestions:

  1. I believe this was already tried but there were some issues and that is why xdotools is being used now. It was before I helped though, so I am not sure what exactly it was. @pythoneer might know more about it. I too was interested in this though. Maybe https://crates.io/crates/x11rb would be a good candidate to use. We already have an issue for this: https://github.com/enigo-rs/enigo/issues/112
  2. I am not sure if that is a good idea. I think we should stick to the names that are being used by the OSs. Otherwise it can be confusing to have the same functionality. It would probably be best to open a separate issue for it so we can discuss it more in depth.

It would be a huge help, if you were able to help with closing https://github.com/enigo-rs/enigo/issues/141. Being able to run integration tests on all platforms would help a lot to avoid regressions

pentamassiv avatar Mar 26 '23 00:03 pentamassiv

@uwejan Hello and welcome! Regarding 1. our first implementation was without xdo but we changed to that library deliberately as to much problems piled up with our own implementation and we couldn't figure out what the problem was. You can take a look at the code here in this PR https://github.com/enigo-rs/enigo/pull/42 changed files how it was earlier which is based on the findings here https://stackoverflow.com/questions/44313966/c-xtest-emitting-key-presses-for-every-unicode-character . Personally i find that many parts in X are just not very well documented (as seen in the stackoverflow post) and things are just behaving funny at times and you need to have "tribal knowledge" to get things to work. I am just warning you that things may look easy but break in funny ways. One thing i remember was hand tuning "sleeps" as sending keys "to fast" just resulted in missing chars in the good case or even total nonsense characters. I always wanted to look at the code of xdo directly and "copy" what they did, but never had the time nor energy to do so.

pythoneer avatar Mar 26 '23 10:03 pythoneer

We should probably bump the version number to 0.1.0. Otherwise we break peoples builds when they run cargo update. I just blindly increased the version number without thinking about that :( I guess we should also yank 0.0.15. What do you think?

pentamassiv avatar Mar 26 '23 23:03 pentamassiv

I bumped the minor version number and yanked 0.0.15. Please run cargo publish again @pythoneer . So far there have been only 28 downloads of 0.0.15 in total so I hope there wasn't too much damage done.

pentamassiv avatar Mar 27 '23 00:03 pentamassiv

Nevermind. I published the new version

pentamassiv avatar Mar 27 '23 13:03 pentamassiv

@pentamassiv Why would that break other peoples build? 0.0.14 -> 0.0.15 would be treated by cargo as a Major change, no "auto update" would occur. Please see https://doc.rust-lang.org/cargo/reference/semver.html

This guide uses the terms "major" and "minor" assuming this relates to a "1.0.0" release or later. Initial development releases starting with "0.y.z" can treat changes in "y" as a major release, and "z" as a minor release. "0.0.z" releases are always major changes. This is because Cargo uses the convention that only changes in the left-most non-zero component are considered incompatible.

But that is not a Problem anyway – i think the resurrection justifies that version bump :P

pythoneer avatar Mar 27 '23 16:03 pythoneer

I had to publish another version today because the windows dependency it relied on got yanked. I tagged the commit that was published. I think we should always do that to know exactly which commit was published as what version

pentamassiv avatar Apr 03 '23 12:04 pentamassiv