Dan Cross cross
cross commented on pull request vitrine/wmaker#10 2025-12-12 08:45:43 -05:00
Start addressing some bugs in new Rust code

I had thought at one point that it might be nice to try to make this generic over some type T, and bifurcate usage between c_void and usize, but looking at the sheer volume of places this code is used, I think that would be tedious and premature. It's a bummer that we have to do a lot of pointer casting as a result, but ... c'est la vie.

cross commented on pull request vitrine/wmaker#10 2025-12-12 07:31:51 -05:00
Start addressing some bugs in new Rust code

oh, hmm. No, not really, then. In that case, I'd recommend using the "provenance" based functions, though: .with_addr and so forth, if that's reasonable: https://doc.rust-lang.org/std/ptr/index.h…

cross approved vitrine/wmaker#9 2025-12-12 07:28:39 -05:00
Rewrite WINGs wfont.c in Rust

This is certainly an improvement! I'm a bit surprised there aren't crates providing better bindings to both Xft and pango, but that's a separate matter.

cross commented on pull request vitrine/wmaker#9 2025-12-12 07:23:08 -05:00
Rewrite WINGs wfont.c in Rust

Instead of the comments, why not give these proper names via let bindings?

cross approved vitrine/wmaker#7 2025-12-12 07:17:35 -05:00
Rewrite WUtils notification.c in Rust
cross commented on pull request vitrine/wmaker#10 2025-12-02 13:59:03 -05:00
Start addressing some bugs in new Rust code

Could you use .cast() here instead of as (and similarly elsewhere)?

cross commented on pull request vitrine/wmaker#7 2025-12-02 13:56:43 -05:00
Rewrite WUtils notification.c in Rust

The actions in the match here are repeated verbatim in all cases (I think). Perhaps delegate to a small helper function that implements just that logic?

cross commented on pull request vitrine/wmaker#7 2025-12-02 13:55:16 -05:00
Rewrite WUtils notification.c in Rust

It strikes me that if you used BTreeMap here instead of HashMap, you could make the constructor for NotificationCenter by pub const fn new() and do away with the OnceLock bits in with_global_default, instead using a static Mutex<NotificationCenter>.

cross commented on pull request vitrine/wmaker#7 2025-12-02 13:49:00 -05:00
Rewrite WUtils notification.c in Rust

"...must be guaranteed by code..." (missing the word "by").

cross commented on pull request vitrine/wmaker#7 2025-12-02 13:48:02 -05:00
Rewrite WUtils notification.c in Rust

I'm mildly bummed out that this went away, because it's decently concise, which is nice.

cross commented on pull request vitrine/wmaker#7 2025-12-02 11:20:01 -05:00
Rewrite WUtils notification.c in Rust

Is it just me, or is the formatting here kind of off?

cross approved vitrine/wmaker#3 2025-12-02 11:17:02 -05:00
Rewrite WUtils string.c in Rust
cross approved vitrine/wmaker#4 2025-12-02 11:09:55 -05:00
Rewrite WUtils bagtree.c in Rust
cross commented on pull request vitrine/wmaker#4 2025-12-02 11:09:44 -05:00
Rewrite WUtils bagtree.c in Rust

ok, I think that's fine.

cross approved vitrine/wmaker#6 2025-11-25 16:34:57 -05:00
Rewrite WUtils tree.c in Rust
cross commented on pull request vitrine/wmaker#6 2025-11-25 16:30:11 -05:00
Rewrite WUtils tree.c in Rust

I wonder about extracting this into a small function that wraps an Option or something? Probably not worth it.

cross suggested changes for vitrine/wmaker#3 2025-10-31 11:02:43 -04:00
Rewrite WUtils string.c in Rust
cross commented on pull request vitrine/wmaker#3 2025-10-31 11:02:43 -04:00
Rewrite WUtils string.c in Rust

I don't think this is what you want, necessarily. You want to find a zero byte, sure, but looking beyond the first len bytes is not useful and may not even be correct, if the source string is not e.g. 0 terminated. I would argue that's a bug, but I could be wrong.

cross commented on pull request vitrine/wmaker#3 2025-10-31 11:02:43 -04:00
Rewrite WUtils string.c in Rust

So this will allocate a Vec<u8> to hold the trimmed bytes, and then invoke alloc_string to do another allocation. I don't know if there's a good way around that; perhaps use strndup? Something like,