GNOME Bugzilla – Bug 787242
Wrap gtk_show_uri_on_window(), e.g. as a member of Gtk::Window
Last modified: 2018-11-04 18:14:31 UTC
I've not found a gtkmm wrapper for gtk_show_uri_on_window, so I need to do e.g. gtk_show_uri_on_window(GTK_WINDOW( m_window.gobj() ), path.c_str(), GDK_CURRENT_TIME, NULL); It'd be handy if this was wrapped, and the most sensible place seems to be as a member function of Gtk::Window, e.g. my_window.show_uri(path, GDK_CURRENT_TIME); // may throw There's also gtk_show_uri(), which would be a member of Gdk::Screen, but as that's deprecated, we probably shouldn't expend time in a way that'd encourage its use.
I agree. That would be good. Thanks.
I presume that, while the GTK+ version takes an optional GError and also returns TRUE/FALSE on success/failure, we would return void and let the user check for exceptions if they care?
Also, regarding the timestamp argument, I presume that: * you'd prefer to express the type as guint32, rather than C++'s std::uint_32_t; * we shouldn't default it to GDK_CURRENT_TIME, as "ideally" it shouldn't be that
(In reply to Daniel Boles from comment #2) > I presume that, while the GTK+ version takes an optional GError and also > returns TRUE/FALSE on success/failure, we would return void and let the user > check for exceptions if they care? Looking around, we seem to just continue returning bool anyway. I guess that makes sense, providing another/simpler way to check for errors.
Created attachment 359315 [details] [review] Window: Wrap gtk_show_uri_on_window() This is a very useful function that gtkmm did not wrap until now. -- Very simple, of course. I didn’t make this const or spend much time thinking about whether we should. But eventually this can lead to e.g. another window getting shown transient-for this one, so probably const would be wrong anyway.
> Looking around, we seem to just continue returning bool anyway. I guess that > makes sense, providing another/simpler way to check for errors. It's true that many glibmm and gtkmm functions both throw an exception and return a bool that shows if it throws an exception. It does not make sense. You can't ignore an exception. If you don't catch it, the program will terminate. You will never see a false return value. Some C++ functions replace the gboolean return value with void. Examples: Gtk::RecentManager::move_item(), Gtk::RecentManager::remove_item(), Gtk::Window::set_icon_from_file(). There are many more examples in glibmm/gio.
Right, yeah. Should we then try to move away from the pattern of returning that bool? (Assuming it always reflects whether the GError was set, rather than being able to reflect other results too.)
Yes, when a gboolean return value is just an alternative to a GError** parameter, IMO the corresponding C++ function should return void.
Created attachment 359664 [details] [review] Window: Wrap gtk_show_uri_on_window() This is a very useful function that gtkmm did not wrap until now. -- Return void instead of bool. -- Shall we open a new bug to ponder removing such bool return values for other functions where they offer nothing?
Review of attachment 359664 [details] [review]: Looks good.
> Shall we open a new bug to ponder removing such bool return values for other functions where they offer nothing? Please feel free to just make those changes in git master.
(In reply to Murray Cumming from comment #10) > Review of attachment 359664 [details] [review] [review]: > > Looks good. Thanks. Just to check, this is OK for gtkmm-3-22, I presume?
I'd rather not, if we can avoid it, please. That would be our first API addition (though not our first deprecation) in the 3.22 branch. Of course, that problem would be solved if there were ever a GTK+ 3.24.
I thought you concluded that since GTK+ have no plans to release that version, reasonable additions could be done in gtkmm-3-22: > On 5 April 2017 at 15:00, Murray Cumming <murrayc@murrayc.com> wrote: > > On Sat, 2017-04-01 at 09:22 +0200, Kjell Ahlstedt wrote: > > > I agree with Diether and Daniel: Let's do as gtk+ does, even > > though > > > they break the rules. > > > > OK. Then I don't object anymore. Thanks for being patient. Feel > > free to > > add API in the gtkmm-3-22 branch: :-)
I have just replied that old email on gtkmm-list to explain.
to master for now Attachment 359664 [details] pushed as cc94bde - Window: Wrap gtk_show_uri_on_window()
I didn't specifically mean to close, but I guess it'll do; we can reopen if we ever start down the slippery slope of adding new, or more accurately missing, API.
(In reply to Kjell Ahlstedt from comment #8) > Yes, when a gboolean return value is just an alternative to a GError** > parameter, IMO the corresponding C++ function should return void. I've opened a bug about the fact that, when replacing this or any other return value with a void, the generated documentation still includes a Returns block with the C description of the ignored return value: Bug 787978
I have pushed Daniel's patch, adding Gtk::Window::show_uri(), to the gtkmm-3-24 branch.
Thanks!