GNOME Bugzilla – Bug 783193
Adapt to OpenURI api change
Last modified: 2017-05-30 18:24:17 UTC
the openuri portal no longer allows file: uris in OpenURI. Instead, it now has a separate OpenFile method for handling local files. The https://git.gnome.org/browse/glib/log/?h=open-file branch has the necessary changes for the glib side.
Created attachment 352764 [details] [review] Use OpenFile for local files The OpenURI portal has a separate method to handle local files now. Use it. At the same time, split out the openuri helpers into separate files, and generate code for the OpenURI portal.
Review of attachment 352764 [details] [review]: A few small issues. ::: gio/gappinfo.c @@ +798,3 @@ if (!res && glib_should_use_portal ()) { + const char *parent_window = NULL; Double space in `const char`. ::: gio/gopenuriportal.c @@ +90,3 @@ + if (!init_openuri_portal ()) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, As with bug #783130, G_IO_ERROR_NOT_INITIALIZED might be more appropriate. But consistency is more important, so go with whatever the decision is on bug #783130. @@ +101,3 @@ + { + char *path; + GUnixFDList *fd_list; I’d initialise these both to NULL to make it a bit clearer that we have ownership of them. @@ +106,3 @@ + path = g_file_get_path (file); + + fd = open (path, O_PATH | O_CLOEXEC); g_open() @@ +111,3 @@ +#endif + fd_list = g_unix_fd_list_new_from_array (&fd, 1); + fd_id = 0; You probably want to clear fd to -1 here, since ownership has transferred into the fd_list. It would make the ownership transfer more obvious. @@ +159,3 @@ + g_variant_get (parameters, "(u@a{sv})", &response, NULL); + + if (response == 0) Is there an enum with these response values somewhere? @@ +215,3 @@ + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) Various comments from g_openuri_portal_open_uri() apply to this function too.
Created attachment 352789 [details] [review] Use OpenFile for local files The OpenURI portal has a separate method to handle local files now. Use it. At the same time, split out the openuri helpers into separate files, and generate code for the OpenURI portal.
Review of attachment 352789 [details] [review]: ::: gio/gopenuriportal.c @@ +167,3 @@ + g_variant_get (parameters, "(u@a{sv})", &response, NULL); + + if (response == XDG_DESKTOP_PORTAL_SUCCESS) This could be turned into a switch statement on the enum now, so we can take advantage of -Wswitch-enum guarantees. @@ +235,3 @@ + g_set_error (&error, G_IO_ERROR, G_IO_ERROR_NOT_INITIALIZED, + "OpenURI portal is not available"); + g_task_report_error (NULL, callback, user_data, NULL, error); This can be simplified using g_task_report_new_error(). @@ +257,3 @@ + + path = g_file_get_path (file); + fd = open (path, O_PATH | O_CLOEXEC); g_open()
Created attachment 352830 [details] [review] Use OpenFile for local files The OpenURI portal has a separate method to handle local files now. Use it. At the same time, split out the openuri helpers into separate files, and generate code for the OpenURI portal.
Review of attachment 352830 [details] [review]: ++
Attachment 352830 [details] pushed as 4c8ab22 - Use OpenFile for local files