Move objc dependency to icrate

[paulora2405]

I'm opening this PR related to #249, but as it's my first contribution, I wanna take time to clear some doubts.

On macos_impl.rs:468 I had to add a allow directive because using the icrate crate, I'm no longer required to create a Class, and thus, apparently cannot fail the NSEvent call, so I unnecessarily return a result from an infallible code. I'm not really sure this is the best approach.

Another thing is, I've opted to go with icrate instead of objc2 directly, this is because, the code didn't really need that low of an access to Objective-C, so I figured that the better ergonomics of icrate would benefit the project in terms of simplicity.

One more thing is, should the usage of the core_graphics crate be kept? I'm not 100% sure, but I think that the objc2 project (not the crate) has a substitute for this crate as well. If that's true, maybe I could give it a go trying to switch to it too.

I've also fixed the timer.rs example, which had an target specific key being used in a general way.

[paulora2405]

I cannot figure out what is wrong with my rustfmt. Lines 226 to 232 of macos_impl.rs were reformatted by VSCode, but now they are the reason CI nightly lints are failing.

So I've tried overriding rustup to use nightly for the project and tried reformatting the file, only to encounter some weird errors with closing multi-line comment */ being inserted after an already closed comment:

[pentamassiv]

I don't know why it is not working for you. I using VS Codium with rust-analyzer v0.3.1740 and for me it works without any issues. It changes the formatting of your PR as you would expect. Are you maybe still using the old Rust extension or an old rust-analyzer version? Those are just guesses though. I never had a problem with the auto-formatting.

[pentamassiv]

You could also try a rustup update, cargo update and cargo fmt. Usually if I have weird issues in VS Codium, a cargo clean fixes it.

[paulora2405]

I'm using the same rust-analyzer version (v0.3.1740), and already updated rustup and cargo, but it made no difference. cargo fmt only throws some warnings about a nightly only features (Warning: can't set `wrap_comments = true`, unstable features are only available in nightly channel.), but doesn't actually reformat any files, and cargo fmt -- --check does not return anything.

[pentamassiv]

TLDR; You need to use Rust nightly for this project.

Okay, that makes sense. The warnings are there because of the content of https://github.com/enigo-rs/enigo/blob/main/rustfmt.toml. On the nightly channel, comments are automatically wrapped so that you don't have to think about entering line breaks yourself. On the stable channel, this is not possible, hence the warnings. I just set my default to nightly with rustup default nightly. Then everything should work as expected.
Sometimes there are differences between the formatting stable does and what nightly expects. That is why in the Github Workflow the formatting check is disabled when not using nightly. cargo fmt formats the code. cargo fmt -- --check only checks if the formatting is correct. Since you apparently are using the stable channel, it formats it just fine. It is just different from what nightly expects.

[paulora2405]

TLDR; You need to use Rust nightly for this project.

Okay, that makes sense. The warnings are there because of the content of https://github.com/enigo-rs/enigo/blob/main/rustfmt.toml. On the nightly channel, comments are automatically wrapped so that you don't have to think about entering line breaks yourself. On the stable channel, this is not possible, hence the warnings. I just set my default to nightly with rustup default nightly. Then everything should work as expected. Sometimes there are differences between the formatting stable does and what nightly expects. That is why in the Github Workflow the formatting check is disabled when not using nightly. cargo fmt formats the code. cargo fmt -- --check only checks if the formatting is correct. Since you apparently are using the stable channel, it formats it just fine. It is just different from what nightly expects.

It is truly bugged it seems, I'm on the latest (nightly) version of rustc and rustfmt and here is the output of a rustfmt --check (which is the same of cargo fmt -- --check):

The change that the nightly CI pipeline indicated is not even one of these. And on stable, there are not formatting changes whatsoever (which could be valid because of new versions) as you mentioned.

[paulora2405]

@pentamassiv I reformatted the code with nightly rustfmt except for the buggy closing comment wrap, which I removed manually.

[pentamassiv]

Hmm, I don't know what is going on. Works on my machine ;-)
I squashed your two commits and ran cargo fmt on it. Now it passes all tests (the one failed test for Rust 1.65 on Ubuntu is okay. It is just a clippy lint).

Did you test if your changes still work?

[pentamassiv]

Sorry, I force pushed your branch to fix the issue. Do you want me to do it again so we are not stopped by the tooling? Now there are a few more commits and it is getting confusing ^^

[paulora2405]

I did test the changes, I ran all the examples and some variations of them, and even noticed one of them was missing a target conditional, which I included in the changes.

But I think it could benefit from testing in more exotic scenarios, I just can't think of any out of the top on my head. If you have a suggestion of a more robust macOS scenario for testing, I would appreciate it.

[paulora2405]

Sorry, I force pushed your branch to fix the issue. Do you want me to do it again so we are not stopped by the tooling? Now there are a few more commits and it is getting confusing ^^

I am very sorry, I clicked to sync my local repository in VSCode and didn't think about how it would push the changes again.

You can force push again, yes, that would be ideal.

[pentamassiv]

Alright, cool. I'll do that and merge it afterwards. Thank you for your PR :)

Testing is a big issue with this crate. That's a problem on all platforms. There aren't many tests yet. There are some involving the browser to get feedback which keys were pressed/released and what the mouse did. The code is messy, it tests very little and we can only run it on Ubuntu with the Github Action. I'd love to fix that and be able to run the tests on macOS and Windows too. Then I would also put more effort into adding test cases. I wrote about some of the stuff I tried in #141. I paused my attempt of getting it to run for now. It is very annoying to do, because of the lack of insight you get from the Github runner. Maybe now that there is logging, it is easier. If you'd like a challenge after fixing the media keys on macOS, feel free to give it a try. It's one of the more difficult issue but would yield the biggest benefit.

[paulora2405]

Alright, cool. I'll do that and merge it afterwards. Thank you for your PR :)

Testing is a big issue with this crate. That's a problem on all platforms. There aren't many tests yet. There are some involving the browser to get feedback which keys were pressed/released and what the mouse did. The code is messy, it tests very little and we can only run it on Ubuntu with the Github Action. I'd love to fix that and be able to run the tests on macOS and Windows too. Then I would also put more effort into adding test cases. I wrote about some of the stuff I tried in #141. I paused my attempt of getting it to run for now. It is very annoying to do, because of the lack of insight you get from the Github runner. Maybe now that there is logging, it is easier. If you'd like a challenge after fixing the media keys on macOS, feel free to give it a try. It's one of the more difficult issue but would yield the biggest benefit.

I've bookmarked the CI issue, I'll for sure try to tackle it in the future, thanks for the patience and mentoring in the first contribution. 😄

[pentamassiv]

Sure, no problem. What happens if you format the code now? Does it still try to change it? It should not make any changes now.

I noticed that you were working on your main branch. You might want to create a branch if you are working on a PR. That way it will be easier to rebase and if you have multiple things you are working on, you don't mix them. I also often forget doing that though

[paulora2405]

Just cloned the og repo, and tried reformatting with stable, but now nothing changes.

On nightly the same problem is happening, beyond the random changes, a closing wrap comment symbol keeps getting inserted, which breaks compilation. I'm not sure what is wrong, I will try to open an issue on https://github.com/rust-lang/rustfmt when I have the time.

[paulora2405]

Looks like there is a issue which seems related to this problem of mine: rust-lang/rustfmt#5964.