-
Notifications
You must be signed in to change notification settings - Fork 7
Description
Currently trying out Niri to test out this PR, few things I have to say at first glance:
-
Don't use a hashtable for event handlers, a simple switch case is a lot more readable.
-
You don't have to warn on unknown events, just simply ignore in the default switch case. If a future version of Niri has a new event, users can handle it using the
eventsignal until we push an update. -
Have the IPC class emit the events in the form of
public signal void event(string event, Json.Node payload)which the Niri class handles in a switch case.- I'm not sure if it would be convenient or not, but we could also define each event payload as a Vala class and rely on `Json.gobject_deserialize
-
Don't expose the event signal with a
Jsonobject as a parameter. Theeventsignal of the Niri class should look likevoid event(string event, string payload), so that users can handle json as they like, for example in python and js using the json module. -
I haven't read the IPC doc of Niri yet so I'm not sure if structs need to be public, but we should prefer them to be internal. For example if an action takes some arguments which are structs we should instead expose its fields as positional arguments.
-
The
msgclass should be merged with the Niri class imo. -
In the
msgclass avoid using delegates, its very hard follow. You should do it in an OOP style instead. Again, it might be better to rely on Json serialize and deserialize functions instead of manually doing it. -
Don't open a new socket connection on each action. Action functions shouldn't be static methods but instance methods that use the socket connection of the Niri singleton object.
Remember that the top priority is to make it easy and convinient to use from language bindings so avoid exposing structs and Json. If a struct has to be exposed, expose them as classes.
Originally posted by @Aylur in Aylur#70 (comment)