Skip to content

All TermControl/TermApp/TermConnection event handlers should be TypedEventHandlers #1104

@DHowett-MSFT

Description

@DHowett-MSFT

WinRT design guidelines dictate that event handlers should have two arguments, T and EventArgs. For any class T that produces an event, this ensures that the event handler can identify which instance of T fired the currently-handled event.

The signature for an event handler delegate should consist of two parameters: sender (IInspectable), and args (some event argument type, for example RoutedEventArgs).

Given that we are exposing an internal API, we can specify a more specific T than IInspectable.

C++/WinRT supports this by way of winrt::TypedEventHandler<T, Args...>.

We should move all the TermConnection, TermApp and TermControl event handlers to use TypedEventHandlers.

bonus

We can get rid of DECLARE_EVENT and DEFINE_EVENT, and augment DECLARE_EVENT_WITH_TYPED_HANDLER (and DEFINE...TYPED) to use typename class_type as the sender type, so that we don't need to repeat ourselves:

#define DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(name, eventHandler, sender, args) \
public: \
winrt::event_token name(Windows::Foundation::TypedEventHandler<sender, args> const& handler); \
void name(winrt::event_token const& token) noexcept; \
private: \
winrt::event<Windows::Foundation::TypedEventHandler<sender, args>> eventHandler;

-#define DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(name, eventHandler, sender, args) \
+#define DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(name, eventHandler, args) \
     public: \
-    winrt::event_token name(Windows::Foundation::TypedEventHandler<sender, args> const& handler); \
+    winrt::event_token name(Windows::Foundation::TypedEventHandler<typename class_type, args> const& handler); \     void name(winrt::event_token const& token) noexcept; \
     private: \
-    winrt::event<Windows::Foundation::TypedEventHandler<sender, args>> eventHandler;
+    winrt::event<Windows::Foundation::TypedEventHandler<typename class_type, args>> eventHandler;

which, when used, becomes:

-DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(PasteFromClipboard, _clipboardPasteHandlers, TerminalControl::TermControl, TerminalControl::PasteFromClipboardEventArgs);
+DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(PasteFromClipboard, _clipboardPasteHandlers, TerminalControl::PasteFromClipboardEventArgs);

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area-TerminalControlIssues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.)In-PRThis issue has a related PRIssue-TaskIt's a feature request, but it doesn't really need a major design.Needs-Tag-FixDoesn't match tag requirementsProduct-TerminalThe new Windows Terminal.

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions