Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event Wrapper Feature Flag #218

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Event Wrapper Feature Flag #218

wants to merge 13 commits into from

Conversation

TTWNO
Copy link
Member

@TTWNO TTWNO commented Dec 17, 2024

  • Move wrappers from atspi-common/src/events/* into atspi-common/src/events/wrapper/*
    • Cache
    • Documents
    • Focus
    • Keyboard
    • Mouse
    • Object
    • Terminal
    • Window
  • Move all macros/impls related to dispatch and wrapper types into the new modules.
    • Cache
    • Documents
    • Focus
    • Keyboard
    • Mouse
    • Object
    • Terminal
    • Window
  • Feature-gate the new wrapper types, their tests, and the macros

@TTWNO TTWNO marked this pull request as draft December 17, 2024 02:18
@TTWNO
Copy link
Member Author

TTWNO commented Dec 17, 2024

cc @luukvanderduim

Let me know if this is similar to your idea. Best example is Document. Lots of clippy, etc. to fix but just want to make sure I'm on the right path.

@luukvanderduim
Copy link
Collaborator

cc @luukvanderduim

Let me know if this is similar to your idea. Best example is Document. Lots of clippy, etc. to fix but just want to make sure I'm on the right path.

Could the following work?

Move these enumeration types into a event_wrapper.rs
Also move all implementations on these enums in this module.
But let's not move end-user facing types in here because those should always be visible.

 `enum Event` 
         `enum DocumentEvents`.
	`enum FocusEvents`
	`enum KeyboardEvents`
	`enum MouseEvents`
	`enum ObjectEvents`
	`enum TerminalEvents`
	`enum WindowEvents`
	`enum AvailableEvent`
	`enum CacheEvents`
	`enum EventListenerEvents`

also add all implementations on above types, such as:

From<&Message> for CacheEvents

and (should we have it):

From<Event> for DocumentEvents

even our big:

From<&Message> for Event

But, for instance not:

From<CacheEvents> for CacheAddEvent

People could want to opt out of Event and have a MessageStream loop and do a

// pseudo rust
while Some(Ok(msg)) = stream.next()? {
   select! {
         CacheAddEvent::try_from(msg)? => got_cache_add(add);
         CaretMovedEvent::try_from(msg)? => caret_moved(moved);
   }
}

There is likely a thousand things wrong with above, but the gist is one could write this and not have Event around.

@TTWNO
Copy link
Member Author

TTWNO commented Dec 17, 2024

Consider it done. Working on it now.

Copy link

codecov bot commented Dec 17, 2024

Codecov Report

Attention: Patch coverage is 98.08841% with 16 lines in your changes missing coverage. Please review.

Project coverage is 85.85%. Comparing base (28658f4) to head (576ca7d).

Files with missing lines Patch % Lines
atspi-common/src/events/event_wrappers.rs 98.36% 13 Missing ⚠️
atspi-common/src/events/traits.rs 92.50% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #218      +/-   ##
==========================================
+ Coverage   83.87%   85.85%   +1.98%     
==========================================
  Files          43       42       -1     
  Lines        3231     3154      -77     
==========================================
- Hits         2710     2708       -2     
+ Misses        521      446      -75     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

TTWNO and others added 3 commits December 17, 2024 13:07
…ype`

Macro `impl_try_from_event_for_user_facing_event_type` does not implement
trait for user facing type at all.

Now named: `impl_try_from_event_for_interface_enum`
@TTWNO TTWNO marked this pull request as ready for review December 30, 2024 22:32
@TTWNO TTWNO requested a review from luukvanderduim December 30, 2024 22:32
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.

Move Event wrapper and its associated traits and impls to a separate module
2 participants