Improvements - Githubissues

HsuJv / vnc-rs

An asynchronous implementation of VNC client side protocol

Apache License 2.0

24 stars 6 forks source link

Improvements #6

Closed wbenny closed 1 year ago

wbenny commented 1 year ago

Hi there! I've stumbled upon your nifty library and it's been real help.

However, I found that I had to do few changes for my use-case. I feel those changes are general enough that they can be useful for everyone.

make auth_method Send + Sync + 'static & make VncConnector & VncState Send/Sync friendly: this is a main change and a must if you'd like to build VncClient e.g. from within tokio::spawn(), which requires Futures to be Send + 'static
get rid of anyhow crate: anyhow isn't really supposed to be used in libraries - it makes capturing library-specific errors quite hard, so the code has been adjusted to return VncError
implement recv_event() method: polling is cool, but I think recv-like method is more async-idiomatic :)

HsuJv commented 1 year ago

Hi @wbenny

Looks great!! But would you mind helping fix the related ci-report before it is merged? Thanks in advance for your contribution

BRs.

wbenny commented 1 year ago

My bad, looks like I've commited files before actualy saving them in the editor. Let's see if it passes now :)

wbenny commented 1 year ago

Huh, looks like I need to do more fixes :)

HsuJv commented 1 year ago

Well, it seems some example code still using anyhow, it's better to add this crate in 'dev-depencies' Also an old declaration of my function breaks the clippy test, can you help remove it so that this PR can be processed with CI happy?

HsuJv commented 1 year ago

For the wasm32 one, I can check it later and provide some help. But if you have any ideas, please feel free to patch it. I've changed the privilege of CI-running so that you can run it yourself. Besides, you can also use cargo build --target wasm32-unknown-unknown --verbose --all-features to test the wasm32 build locally.

BRs

wbenny commented 1 year ago

I've managed to fix everything mentioned and force-pushed over the last commit. However, it looks like the CI workflow didn't trigger.

HsuJv commented 1 year ago

Cheers! Happy to hear that.

Do you need a new release of this? If so I will process it ASAP.

wbenny commented 1 year ago

New release would be perfect :)

Thank you!

HsuJv commented 1 year ago

Hi there, I've just verified that this PR not breaking the current work on both Ubuntu 20.04 and WASM for the browser. And publish version 0.5.0 as there are API changes. BRs.