floooh / sokol-zig

Zig bindings for the sokol headers (https://github.com/floooh/sokol)
zlib License
355 stars 49 forks source link

Wrong type for `getClipboardString()` #21

Closed JustinRyanH closed 2 years ago

JustinRyanH commented 2 years ago

When calling getClipboardString zig will fail to compile.

./lib/sokol-zig/src/sokol/app.zig:380:37: error: expected type '[:0]const u8', found '[*c]const u8'
    return sapp_get_clipboard_string();
                                    ^

A local solution I've found is to change the return type of getClipboardString() to [*c]const u8 and then run std.mem.span(sapp.getClipboardString()) as the result.

Since Sokol-zig is a autogenerated wrapper for Sokol the main reason I'm creating this issue is to get guidance on what sort of direction we want to create the fix to keep it inline with the spirit of the project. Once I receive that guidance I'll create a PR in the main floooh/sokol repository based on that response.

floooh commented 2 years ago

Good catch, I should create a compile test for the zig-bindings which at least call all the API functions, so that problems like this show up earlier.

I wonder if there's just a @ptrCast missing (because that's how it works in the other direction from [:0]const u8 to [*c]const u8. I'll tinker around a bit.

Don't worry about a PR, I'm currently working on the language bindings stuff anyway, and will fix this while at it (I guess Ill start with creating some API tests though).

PS: a simple pointer cast doesn't work of course, because the result is a slice :)

floooh commented 2 years ago

...looks like std.mem.span() works directly on the C pointer:

pub extern fn sapp_get_clipboard_string() [*c]const u8;
pub fn getClipboardString() [:0]const u8 {
    return @import("std").mem.span(sapp_get_clipboard_string());
}

Before that I tried to first cast the [*c]const u8 to a sentinel-terminated pointer:

const ptr = @ptrCast([*:0]const u8, sapp_get_clipboard_string());

...and then call std.mem.span() on that ptr, but it seems to be unnecessary.

When I then call sapp.getClipboardString() with the string 'zig build run-quad' in the clipboard, it seems to be correct:

    const str = sapp.getClipboardString();
    @import("std").debug.print("str: '{s}', len: {}, type: {s}\n", .{ str, str.len, @typeName(@TypeOf(str)) });

...this prints

str: 'zig build run-quad', len: 18, type: [:0]const u8

...I'll try to add that "transformation" for all C-string-slice conversions in the bindings code generator (I'll probably wrap it in a little helper cStrToSlice() helper functions.

floooh commented 2 years ago

Ok, I have updated the bindings, also some unrelated changes because the sokol headers hadn't been updated for a little while.

The important part is that each Zig wrapper now has a helper function like this to convert a C string to a Zig string slice:

https://github.com/floooh/sokol-zig/blob/f9f9709a4e34ad59396b4102ea6d04d45f6d2496/src/sokol/app.zig#L3-L6

...and this is used in two places (these are all cases of functions with a C-string result):

https://github.com/floooh/sokol-zig/blob/f9f9709a4e34ad59396b4102ea6d04d45f6d2496/src/sokol/app.zig#L390-L393

https://github.com/floooh/sokol-zig/blob/f9f9709a4e34ad59396b4102ea6d04d45f6d2496/src/sokol/app.zig#L406-L409

PS: tested with Zig version 0.10.0-dev.4367+800edb03b

JustinRyanH commented 2 years ago

@floooh Oh sweet. Thanks for addressing this!