webui-dev / v-webui

Use any web browser as GUI, with V in the backend and modern web technologies in the frontend.
https://webui.me
MIT License
112 stars 11 forks source link

Refactor get args #66

Closed ttytm closed 1 year ago

ttytm commented 1 year ago

This PR moves towards a uniform get_arg function for JS argument parsing for this wrapper. The main advantages is that handling of null / error values is simplified and emphasised to make code safer. The more uniform, explicit way to access JS arguments might be seen as another small advantage.

The text-editor examples open function can be used to represent the advantages of the added result return in a real application.

Alternatively, we could add the get_arg function and keep the string(), event.int(), event.bool() methods to parse args with default values. But since V enables the improved saftey in a fairly simple way, disallowing the more unsafe code can be a good choice for a library. What do you think @hassandraga?

hassandraga commented 1 year ago

I like the new e.get_arg[ T ]() method, but how are you planning to implement indexes after we finish Supporting variable number of arguments?

ttytm commented 1 year ago

Nice. Thanks for the review!

I'd simply add the index.

Assumed the get functions become:

WEBUI_EXPORT const char* webui_get_string(webui_event_t* e, size_t index);
// get_arg parses the JavaScript argument at the specified index into a V data type.
pub fn (e &Event) get_arg[T](idx usize) !T {
    if e.size == 0 {
        element := unsafe { (&char(e.element)).vstring() }
        return error('`${element}` did not receive a `${T.name}` argument.')
    }
    return $if T is int {
        int(C.webui_get_int(e, idx))
    } $else $if T is i64 {
        C.webui_get_int(e, idx)
    } $else $if T is string {
        unsafe { (&char(C.webui_get_string(e, idx))).vstring() }
    } $else $if T is bool {
        C.webui_get_bool(e, idx)
    } $else {
        json.decode(T, (&char(C.webui_get_string(e, idx))).vstring()) or {
            return error('Failed decoding `${T.name}` argument. ${err}')
        }
    }
}

Or with a params struct. Then it would default to 0 and an index could be specified optionally. get_arg[string]() would work, get_arg[string](idx: 1) would work. It also wouldn't introduce another breaking change after the get_arg implementation.

[params]
pub struct GetArgOptions {
    idx usize
}

// get_arg parses the JavaScript argument into a V data type.
// To decode JS functions with multiple arguments, the argument index can be passed.
// Example: get_arg[strnig](idx: 1)
pub fn (e &Event) get_arg[T](opts GetArgOptions) !T {
    if e.size == 0 {
        element := unsafe { (&char(e.element)).vstring() }
        return error('`${element}` did not receive a `${T.name}` argument.')
    }
    return $if T is int {
        int(C.webui_get_int(e, opts.idx))
    } $else $if T is i64 {
        C.webui_get_int(e, opts.idx)
    } $else $if T is string {
        unsafe { (&char(C.webui_get_string(e, opts.idx))).vstring() }
    } $else $if T is bool {
        C.webui_get_bool(e, opts.idx)
    } $else {
        json.decode(T, (&char(C.webui_get_string(e, opts.idx))).vstring()) or {
            return error('Failed decoding `${T.name}` argument. ${err}')
        }
    }
}
hassandraga commented 1 year ago

Good then, I will try to implement it soon. Yes, the default will be 0.