Created attachment 192736 [details] [review]
introspection: add missing introspection data from Vala bindings

Comment on attachment 192736 [details] [review]
introspection: add missing introspection data from Vala bindings

>+ * @data: (closure): the user data that was passed to
>  * soup_address_resolve_async()

if this parameter was renamed to "user_data", would that have the same effect?

>+/**
>+ * soup_content_sniffer_sniff:
>+ * @sniffer: the sniffer
>+ * @msg: message to sniff
>+ * @buffer: buffer
>+ * @params: (element-type utf8 utf8) (out) (transfer full) (allow-none): parameters
>+ **/

Let's document this properly while we're here:

/**
 * soup_content_sniffer_sniff:
 * @sniffer: a #SoupContentSniffer
 * @msg: the message to sniff
 * @buffer: a buffer containing the start of @msg's response body
 * @params: (element-type utf8 utf8) (out) (transfer full) (allow-none): return
 *   location for Content-Type parameters (eg, "charset"), or %NULL
 *
 * Sniffs @buffer to determine its Content-Type. The result may also be influenced
 * by the Content-Type declared in @msg's response headers.
 *
 * Return value: the sniffed Content-Type of @buffer; this will never be %NULL,
 *   but may be "application/octet-stream".
 */

>+ * Return value: (transfer full) (element-type utf8): a #GSList of
>+ * list elements, as allocated strings
>  **/
> GSList *
> soup_header_parse_list (const char *header)

This (and the other annotations in soup-headers.c) is wrong; the returned strings are not guaranteed to be UTF-8. However, they're almost always ASCII in reality, so this is probably more useful than leaving them un-annotate. (I thought there was a bug about fixing the APIs, but I don't see one...)

>+/**
>+ * soup_proxy_resolver_get_proxy_sync:

SoupProxyResolver is deprecated in favor of SoupProxyURIResolver, and there's no reason for anyone to use it, so remove this.

>  * soup_proxy_uri_resolver_get_proxy_uri_sync:
>  ...
>+ * @cancellable: (allow-none): a #GCancellable, or %NULL

GCancellables do not need to be marked allow-none.

>  * soup_socket_connect_async:

likewise here, and throughout soup-socket.c

(In reply to comment #2)
> (From update of attachment 192736 [details] [review])
> >+ * @data: (closure): the user data that was passed to
> >  * soup_address_resolve_async()
> 
> if this parameter was renamed to "user_data", would that have the same effect?

Yes.

> >+/**
> >+ * soup_content_sniffer_sniff:
> >+ * @sniffer: the sniffer
> >+ * @msg: message to sniff
> >+ * @buffer: buffer
> >+ * @params: (element-type utf8 utf8) (out) (transfer full) (allow-none): parameters
> >+ **/
> 
> Let's document this properly while we're here:
> 
> /**
>  * soup_content_sniffer_sniff:
>  * @sniffer: a #SoupContentSniffer
>  * @msg: the message to sniff
>  * @buffer: a buffer containing the start of @msg's response body
>  * @params: (element-type utf8 utf8) (out) (transfer full) (allow-none): return
>  *   location for Content-Type parameters (eg, "charset"), or %NULL
>  *
>  * Sniffs @buffer to determine its Content-Type. The result may also be
> influenced
>  * by the Content-Type declared in @msg's response headers.
>  *
>  * Return value: the sniffed Content-Type of @buffer; this will never be %NULL,
>  *   but may be "application/octet-stream".
>  */
> 
> >+ * Return value: (transfer full) (element-type utf8): a #GSList of
> >+ * list elements, as allocated strings
> >  **/
> > GSList *
> > soup_header_parse_list (const char *header)

Awesome, thank you!

> This (and the other annotations in soup-headers.c) is wrong; the returned
> strings are not guaranteed to be UTF-8. However, they're almost always ASCII in
> reality, so this is probably more useful than leaving them un-annotate. (I
> thought there was a bug about fixing the APIs, but I don't see one...)
> 
> >+/**
> >+ * soup_proxy_resolver_get_proxy_sync:
> 
> SoupProxyResolver is deprecated in favor of SoupProxyURIResolver, and there's
> no reason for anyone to use it, so remove this.

Wouldn't it make more sense to add a "Deprecated: 2.36: Use SoupProxyURIResolver instead" to the documentation instead of just removing it? (2.36 is probably wrong, but I have no idea when it was deprecated).

> >  * soup_proxy_uri_resolver_get_proxy_uri_sync:
> >  ...
> >+ * @cancellable: (allow-none): a #GCancellable, or %NULL
> 
> GCancellables do not need to be marked allow-none.

Good to know, thanks.

> >  * soup_socket_connect_async:
> 
> likewise here, and throughout soup-socket.c

(In reply to comment #3)
> Wouldn't it make more sense to add a "Deprecated: 2.36: Use
> SoupProxyURIResolver instead" to the documentation instead of just removing it?

It was never documented before though. (There used to be gtk-doc comments in the file, but they were never linked into the actual documentation.)

(In reply to comment #4)
> (In reply to comment #3)
> > Wouldn't it make more sense to add a "Deprecated: 2.36: Use
> > SoupProxyURIResolver instead" to the documentation instead of just removing it?
> 
> It was never documented before though. (There used to be gtk-doc comments in
> the file, but they were never linked into the actual documentation.)

Leaving it as is means it will still appear in the GIR, just with incorrect information... I doubt that's what you want.

It seems like you want the API hidden in both the GIR and the documentation. Is that right? Is it just this function, or the entire SoupProxyResolver API?

(In reply to comment #5)
> It seems like you want the API hidden in both the GIR and the documentation. Is
> that right? Is it just this function, or the entire SoupProxyResolver API?

The entire API; it was deprecated before g-i was ever available in libsoup, so there's no reason for anyone to have ever used it via g-i. I've just pushed a Makefile.am fix to remove it from the gir.

Created attachment 193207 [details] [review]
Soup-2.4.gir: add missing introspection data from Vala bindings

I believe this resolves everything.

pushed to master. Thanks!

Attachment 193207 [details] pushed as 08d2505 - Soup-2.4.gir: add missing introspection data from Vala bindings