这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@alextremblay
Copy link
Contributor

Update documentation as per #42 (comment)

Copy link
Owner

@Narsil Narsil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we just keep this modification to adding the example for unstable_grab ?

Your example seems broken too as you are not returning the event if it's not a CapsLock key event. (You can run cargo test --all-features --all-targets && cargo clippy --all-features --all-targets to check your PR)

Also could you revert all the text modifications to their previous versions:

  • global input device event stream for instance is incorrect both for Windows and MacOS (someone could always be higher up in the chain than you for instance). (And it's not the README's place to dive into OS specific considerations).
  • Also OS specificities need to be higher than the unstable_ feature, because well unstable is unstable so it's by definition less important than the rest.
  • also the listen on linux does not use evdev but real X11 mechanism (so no special permissions needed).

Caveatsis probably better thanSpecificities` though.

@alextremblay
Copy link
Contributor Author

Thanks @Narsil,

Following your suggestion, I cleaned up the documentation and split the "OS Caveats" section into a separate section for each of the listen and grab functions, making them more prominent and explicit

I've run the cargo tests and clippy checks you suggested on windows and MacOS, and all the tests pass.

Could you please review?

@alextremblay alextremblay requested a review from Narsil June 4, 2021 18:18
Copy link
Owner

@Narsil Narsil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM.

@Narsil
Copy link
Owner

Narsil commented Jun 7, 2021

In order to pass quality, you should change the lib.rs file first, then run cargo readme > README.md.

@alextremblay
Copy link
Contributor Author

you mean do them as two separate commits?

also, on windows 10, cargo readme > README.md converts README.md to a UTF-16 encoded file, which git interprets as a binary file. i had to do cargo readme -o README.md instead.

If i get a chance, i'll open up another PR with some basic contributor guidelines, based on what I've learned so far :)

@alextremblay
Copy link
Contributor Author

not sure how I managed to lose that semicolon, but it's been re-added, and cargo test, cargo clippy, and cargo fmt have all been successfully run

@alextremblay
Copy link
Contributor Author

oh that's an interesting error...

i see in the github ctions workflow file that only the serialize feature is enabled, and there's a note saying that grab is not supported in linux. is this a holdover from before the evdev refactoring, or is grab testing on linux disabled for some other reason?

I just booted up my linux laptop, cloned the repo to it, and ran the doctests with all features enabled, and it passed all tests.

should i add a //! #[cfg(feature = "unstable_grab")] line to each section of the grab example, or should i modify the rust.yaml file to enable unstable_grab in linux?

@Narsil
Copy link
Owner

Narsil commented Jun 8, 2021

Protect the example with #[cfg] grab is still not a by default feature :)

@Narsil Narsil merged commit fd0ca96 into Narsil:master Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants