GNOME Bugzilla – Bug 676313
gvfs-open: use g_app_info_launch_default_for_uri()
Last modified: 2012-06-25 15:45:23 UTC
noticed while trying to test the fix for bug 666386
Created attachment 214306 [details] [review] gvfs-open: use g_app_info_launch_default_for_uri() g_app_info_launch_default_for_uri() didn't exist when gvfs-open was first written, but now that it does, there's no reason not to use it.
Review of attachment 214306 [details] [review]: Looks good to me
belatedly committed Attachment 214306 [details] pushed as e8b4b33 - gvfs-open: use g_app_info_launch_default_for_uri()
Was just looking at it yesterday. The g_app_info_launch_default_for_uri() can't handle local paths, i.e. strings that are not valid URIs. Let's deal with it in another commit, reopening for now.
Created attachment 217024 [details] [review] gvfs-open: Construct full URI for local paths g_app_info_launch_default_for_uri() can't cope with local paths, which are not valid URIs. To work around this, let's go through GFile and let it construct a file:// URI so we don't regress from previous versions. gvfs-open is heavily used by xdg-open.
Comment on attachment 217024 [details] [review] gvfs-open: Construct full URI for local paths >+ file = g_file_new_for_commandline_arg (locations[i]); >+ res = g_app_info_launch_default_for_uri (g_file_get_uri (file), You could just call g_filename_to_uri() (which is what GFile will end up doing internally)...
(In reply to comment #6) > (From update of attachment 217024 [details] [review]) > >+ file = g_file_new_for_commandline_arg (locations[i]); > >+ res = g_app_info_launch_default_for_uri (g_file_get_uri (file), > > You could just call g_filename_to_uri() (which is what GFile will end up doing > internally)... Good point, wasn't aware of that function. However brief testing indicates it's not a full replacement, GFile does much more during its construction, such as path canonicalization. Generally g_filename_to_uri() works well with absolute pathnames but can't cope with relative ones (and URIs are always absolute).
ah. yeah, the patch makes sense to me then
Comment on attachment 217024 [details] [review] gvfs-open: Construct full URI for local paths (In reply to comment #8) > ah. yeah, the patch makes sense to me then Committed to master. Closing this bugreport, feel free to report any issues caused by these changes.