Simplify WINGs handlers API and reimplement in Rust #17

trurl · 2026-02-12T13:43:11-05:00

trurl commented

2026-02-12 13:43:11 -05:00

This PR removes a bit of cruft which is not used by Window Maker (most notably, support for calling handlers in response to input on a file descriptor), massages the API slightly to make it easier to rewrite in safe Rust, and moves support for scheduling callbacks on a global queue into wutil-rs.

This PR removes a bit of cruft which is not used by Window Maker (most notably, support for calling handlers in response to input on a file descriptor), massages the API slightly to make it easier to rewrite in safe Rust, and moves support for scheduling callbacks on a global queue into `wutil-rs`.

trurl added 4 commits 2026-02-12 13:43:12 -05:00

Remove WMDeleteTimerWithClientData. 5075c877fa

This function requires tracking an explict client data pointer, which it would be preferable not to have in the Rust rewrite.

Removing this function entails tracking timer handlers explicitly, but this is not an undue burden.

Drop select(2)-based input FD polling. ce97a3b39f

This is an intermediate step in the process of dropping input FD
polling entirely (except for pumping the X11 event queue).

There is code to use poll(2) instead of select(2) to wait for events
on file descriptors (which includes waiting for events from the X11
event queue). It appears not to have been used in quite some time,
or perhaps never, as it contains a typo (`poll fd` instead of `pollfd`
on its first line).

To reduce code bloat and make source code easier to navigate, it
makes sense to prefer only one of these codepaths.

After correcting this typo, waiting for X11 events appears to work, so
the poll(2)-based code seems to be good enough. Keeping the poll-based
code is vaguely preferable because poll(2) has a slightly better
interface than select(2), and poll(2) has been a standard Unix API
for 30 years.

Since this code is slated to be replaced, the decision to use the
poll(2)-based code here is probably not very important. But future
programmers may want to use this as a branching-off point for switching
to epoll(7) or some other, less-select(2)-like interface.

Remove WINGs input handlers. 223a64286f

Input handlers date back to early versions of WINGs. They provide
a way to listen for state changes on file descriptors (including
networking sockets) as part of the WINGs event loop.

This functionality is not actually used in the Window Maker source
tree. It may have been more useful in the past, when WINGs provided
a networking socket API. The status quo appears to be that this is
largely dead code. Pumping the X11 event queue is special-cased in
the input handling code, so it can stay in for now.

Rewrite WINGs handlers.c in Rust. 0457587fed

trurl requested review from cross 2026-02-12 13:43:24 -05:00

cross reviewed 2026-02-15 11:54:42 -05:00

wutil-rs/src/handlers.rs Outdated

						
				@@ -0,0 +176,4 @@

				    }

				}

				struct TimerHandler {

cross commented

2026-02-15 11:23:57 -05:00

Nit: a couple of do comments on this struct and callback would be nice.

Nit: a couple of do comments on this struct and `callback` would be nice.

trurl commented

2026-02-15 23:14:58 -05:00

Done.

trurl marked this conversation as resolved

wutil-rs/src/handlers.rs Outdated

						
				@@ -0,0 +184,4 @@

				    delay: Duration,

				}

				struct IdleHandler {

cross commented

2026-02-15 11:24:14 -05:00

(Similarly, a comment here would be nice.)

trurl commented

2026-02-15 23:14:55 -05:00

Done.

trurl marked this conversation as resolved

wutil-rs/src/handlers.rs Outdated

						
				@@ -0,0 +200,4 @@

				        mem::swap(&mut idle_handlers, &mut handlers.idle_handlers);

				        idle_handlers

				    });

				    for mut h in handlers.into_iter() {

cross commented

2026-02-15 11:52:55 -05:00

I don't quite get this; it appears to not just check, but consume and drop, global idle handlers. Perhaps the name could be changed to reflect that? run_global_idle_handlers or something? The repeated with_global_handlers blocks are interesting; I believe that's intended for the case where an idle handler is added during processing?

I don't quite get this; it appears to not just check, but consume and drop, global idle handlers. Perhaps the name could be changed to reflect that? `run_global_idle_handlers` or something? The repeated `with_global_handlers` blocks are interesting; I believe that's intended for the case where an idle handler is added during processing?

trurl commented

2026-02-15 23:14:50 -05:00

The name comes from the original C API, but I agree that it's misleading. run_global_idle_handlers is certainly better. Change made.

The repeated with_global_handlers blocks are interesting; I believe that's intended for the case where an idle handler is added during processing?

Yes. The repeated with_global_handlers blocks are written that way in case a callback tries to touch the global queue (which would fail if the callback is invoked from within with_global_handlers because it's inside a mutex). The original C implementation does something similar (copying the global idleHandler array in W_CheckIdleHandlers and iterating over the copy).

The name comes from the original C API, but I agree that it's misleading. `run_global_idle_handlers` is certainly better. Change made. > The repeated with_global_handlers blocks are interesting; I believe that's intended for the case where an idle handler is added during processing? Yes. The repeated `with_global_handlers` blocks are written that way in case a callback tries to touch the global queue (which would fail if the callback is invoked from within `with_global_handlers` because it's inside a mutex). The original C implementation does something similar (copying the global `idleHandler` array in `W_CheckIdleHandlers` and iterating over the copy).

trurl marked this conversation as resolved

wutil-rs/src/handlers.rs Outdated

						
				@@ -0,0 +222,4 @@

				    #[unsafe(no_mangle)]

				    pub unsafe extern "C" fn WMAddTimerHandler(

				        milliseconds: c_int,

cross commented

2026-02-15 11:53:57 -05:00

Is there a good reason not to change the type signature on the C side to make this explicitly unsigned? A negative number there feels relativistically impossible?

trurl commented

2026-02-15 23:14:01 -05:00

I've been trying to keep things compatible with the original headers, but I have definitely broken source-level compatibility. So, yeah, let's use unsigned where appropriate.

I've been trying to keep things compatible with the original headers, but I have definitely broken source-level compatibility. So, yeah, let's use `unsigned` where appropriate.

trurl marked this conversation as resolved

trurl force-pushed refactor/riir.wings-handlers from 0457587fed to 7c7b9aa97c

2026-02-15 23:16:15 -05:00

Compare

trurl merged commit 7c7b9aa97c into refactor/riir

2026-02-19 17:03:26 -05:00

trurl deleted branch refactor/riir.wings-handlers

2026-02-19 17:03:51 -05:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vitrine/wmaker#17

Allow edits from maintainers