Created attachment 55588 [details] [review] glib: various minor introspection improvements I'm working on porting the Vala bindings to GIR, and these improvements are necessary to avoid regressions. I also went ahead and cleaned up all the warnings g-ir-scanner was emitting when --warn-all was added since it was low-hanging fruit.
Comment on attachment 55588 [details] [review] glib: various minor introspection improvements Review of attachment 55588 [details] [review]: ----------------------------------------------------------------- Thanks for the patch! just a few comments ::: glib/poppler-attachment.h @@ +39,2 @@ > * @count: number of bytes in @buf. > + * @data: (closure closure): user data passed to poppler_attachment_save_to_callback() This should be (closure), (closure closure) is the callback. @@ +41,1 @@ > * @error: GError to set on error, or NULL We should probably annotate this as (out), no? ::: glib/poppler-document.cc @@ +2497,4 @@ > * Returns the #PopplerFormField for the given @id. It must be freed with > * g_object_unref() > * > + * Return value: (transfer full): a new #PopplerFormField or NULL if Use %NULL, please. ::: glib/poppler-media.h @@ +38,2 @@ > * @count: number of bytes in @buf. > + * @data: (closure closure): user data passed to poppler_media_save_to_callback() Same here, this should be (closure). @@ +40,1 @@ > * @error: GError to set on error, or NULL And here we could annotate this as (out)
Created attachment 55609 [details] [review] glib: various minor introspection and documentation improvements (In reply to comment #1) > Comment on attachment 55588 [details] [review] [review] > glib: various minor introspection improvements > > Review of attachment 55588 [details] [review] [review]: > ----------------------------------------------------------------- > > Thanks for the patch! just a few comments Thanks for the quick review. > ::: glib/poppler-attachment.h > @@ +39,2 @@ > > * @count: number of bytes in @buf. > > + * @data: (closure closure): user data passed to poppler_attachment_save_to_callback() > > This should be (closure), (closure closure) is the callback. Oops. > @@ +41,1 @@ > > * @error: GError to set on error, or NULL > > We should probably annotate this as (out), no? No, that's not necessary. GObject introspection will do the right thing here. > ::: glib/poppler-document.cc > @@ +2497,4 @@ > > * Returns the #PopplerFormField for the given @id. It must be freed with > > * g_object_unref() > > * > > + * Return value: (transfer full): a new #PopplerFormField or NULL if > > Use %NULL, please. I guess there is no reason not to fix this while fixing something else on the same line... > ::: glib/poppler-media.h > @@ +38,2 @@ > > * @count: number of bytes in @buf. > > + * @data: (closure closure): user data passed to poppler_media_save_to_callback() > > Same here, this should be (closure). Again, oops. > @@ +40,1 @@ > > * @error: GError to set on error, or NULL > > And here we could annotate this as (out) Yes, but it's unnecessary. However s/NULL/%NULL/. Actually, I'll just fix all the results from grep -P '^ \*.+ NULL' *.{cc,h}
Comment on attachment 55609 [details] [review] glib: various minor introspection and documentation improvements Review of attachment 55609 [details] [review]: ----------------------------------------------------------------- Pushed to git master and poppler-0.18. Thank you very much.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.