Zig implementation #11

There's no need for scoping here, this is a single file just use the default scope. (e.g. std.log.err())

Use std.process.fatal() instead.

IMO setting using undefined as the default value for a struct field is poor style, it makes it too easy to forget to initialize the field. Please don't use it as a default struct field value here and elsewhere.

This function is pointless, inline it. Same goes for all the other trivial functions (handleClosed(), handleDimensions(), etc.)

Please add a blank line between function definitions.

Code smell, don't use undefined as a default field value.

Use a tagged union for requested move/resize.

Function is pointless, inline it.

Skip the redundant mem.span() calls and use mem.orderZ()

I don't particularly like having so many global variables. I'd rather have only wm and use e.g. wm.gpa instead. Is the io global even used anywhere currently?

Use the global wm rather than passing a *WindowManager pointer to this function to avoid confusion.

There's no need for a loop here, use river_seat.getUserData() and cast the pointer instead.

Also please inline handlePointerMoveRequested and handlePointerResizeRequested

Use a switch on window.pointer_request rather than consecutive if statements.

should be const

I would write this as catch fatal("OOM"); and define const fatal = std.process.fatal; at the top of the file.

This should likewise be catch fatal("OOM");, the current error message is nonsense.

See comments on the equivalent XkbBinding code above.

Most if not all of this op-related data would be better represented as a tagged union.

seat.hovered is an optional pointer and we want to set seat.hovered to null even if the window has been destroyed. The if statement should be eliminated.

Same as above, eliminate the if statement.

This can be simplified:

while(seat.xkb_bindings.first())|binding|binding.destroy();

Same goes for the other instances of this pattern.

This is code smell. Please refactor this to eliminate the need for the orelse unreachable.

This should not be a fatal error.

This .? assertion is unnecessary, please refactor this to avoid the need (i.e. use a second if statement).

Same below.

Use zig-xkbcommon rather than hardcoding these.

I'd rather translate-c and import the linux/input-event-codes.h header than hardcode these. It would serve better as an example.

should be const

should be const

should be const

the return is redundant.

The "handle" prefix is superfluous, rename these to just manageStart()/renderStart()

Please rename these to e.g. Window.create(ev.id).

The wm variable should be initialized using wm = .{ } syntax to benefit from compile errors when a field is forgotten. It might make sense to inline this init() function.

"OOM"

There is currently Illegal behavior if the server does not advertise the river_window_manager_v1 global.

If the server does not advertise either of the required globals tinyrwm should exit with an error message.

This illegal behavior has not been resolved.

This is a misleading variable name, also river_xkb_bindings_v1 version 3 is technically not required. I'd personally drop these variable names and inline like tinyrwm.c

Leaving width/height undefined beyond the create() function requires some justification IMO. Explain why it is impossible for illegal behavior to happen in a comment or choose a different approach.

Why is this an optional pointer? Same question for resize.

It would be cleaner if these fields were not optional pointers. The wm.obj.? assertions strewn around the code are ugly, and very much avoidable if the code were restructured to only initialize the wm struct after the initial roundtrip is completed for example.

This is not idiomatic Zig, prefer imperative style.

I quite dislike multi-line while/if statements without curlies, https://codeberg.org/river/river/src/branch/main/CONTRIBUTING.md#coding-style