Event handling thoughts

[dodexahedron]

First off, I love that the new project guidelines specify good practices such as use of EventHandler<TArgs> and I also love that it looks like that's been at least mostly followed in changes since v1. 😊

BUT. (I know, I know... "Why does there always have to be a "but," @dodexahedron?")

Well... This one is pretty important, and I wanted to introduce the issue here rather than posting an issue (though I've got plenty typed up if we need to ultimately end up there), since I think intentions may already be in a good place, for v2, and that time, other project needs, and that sort of thing are probably behind a lot of what I want to talk about. So, keep in mind I realize all that! 😅

Anyway....

I wanted to talk a bit more in depth about events and how they're implemented/designed in Terminal.Gui, especially since v2's events (and most of v2 really) are already API-incompatible and thus this is a good time to right the ship.

My biggest concern is around the OnSomeEvent wrappers, which are public and, per design guidelines/best practices, really should not be. The raising of an event and whatever happens around raising that event are type-specific implementation details that only the class explicitly implementing the event should know about.

Anyway... This is the kick-off post.

The main prompt is that all or most of those methods (at least for the non-static events) should really be protected virtual, for a lot of reasons, and this is an ideal time to fix it for v2.

[dodexahedron]

Some documentation that is relevant to this topic and which may or may not be helpful for anyone with thoughts on the matter (or even just for FYI for anyone wanting to lurk):

• https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/events/how-to-raise-base-class-events-in-derived-classes
• https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/event
• https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/event
• https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.control.invoke?view=windowsdesktop-8.0
• https://learn.microsoft.com/en-us/dotnet/api/system.environment.failfast?view=net-8.0#system-environment-failfast(system-string-system-exception)
  • This one is for a specific reason

[tig]

Once again, I appreciate the insights.

I want to be pragmatic about this and directed.

There are already several open issues, which either implicitly or explicitly seek to address design errors around OnEvent code.

The most useful thing that people could do right now is go find those issues and update them to add to do to correct these problems.

In addition, anytime, anyone is in the code, and they see an instance that breaks these design guidelines they should open an issue for it.

[dodexahedron]

Coolio.

Perhaps this is an ideal thing for me to shift my efforts to, then, once I wrap up the Color tests.

[dodexahedron]

Alrighty.

Now that I'm finished with Color, for now, I'll spin up an issue to get to work on that.