After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 738513 - DialogFlags cannot be set for Dialog
DialogFlags cannot be set for Dialog
Status: RESOLVED FIXED
Product: gtkmm
Classification: Bindings
Component: general
3.12.x
Other Linux
: Normal normal
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2014-10-14 10:21 UTC by Simonas Kazlauskas
Modified: 2014-12-13 10:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch: Gtk::Dialog: Add constructors with DialogFlags (2.72 KB, patch)
2014-10-17 17:32 UTC, Kjell Ahlstedt
none Details | Review
patch: Gtk::Dialog: Add constructors with DialogFlags (3.33 KB, patch)
2014-10-19 08:56 UTC, Kjell Ahlstedt
none Details | Review
Test case (1.67 KB, application/x-compressed-tar)
2014-12-13 10:00 UTC, Kjell Ahlstedt
  Details

Description Simonas Kazlauskas 2014-10-14 10:21:25 UTC
You cannot set DialogFlags for Dialog (making functionality of gtk_dialog_new_with_buttons missing in the binding) by using gtkmm. This makes it impossible to set use-header-bar property to true on created dialogs.
Comment 1 Kjell Ahlstedt 2014-10-16 17:25:56 UTC
The use-header-bar property is construct-only. It can't be set with
set_use_header_bar(true) or property_use_header_bar() = true after the
dialog has been created.

Dialog has these constructors:
  Dialog();
  explicit Dialog(const Glib::ustring& title, bool modal = false);
  Dialog(const Glib::ustring& title, Gtk::Window& parent, bool modal = false);

We can't remove any of them, it would break ABI.
We can add new constructors, but it must be done in such a way that no calls
to a Dialog constructor becomes ambiguous.
As an example, it's tempting to add
  explicit Dialog(const Glib::ustring& title, bool use_header_bar = false,
                  bool modal = false);
  Dialog(const Glib::ustring& title, Gtk::Window& parent,
         bool use_header_bar = false, bool modal = false);

Unfortunately, a simple constructor call, such as
  gtkmm::Dialog dialog("Ambiguous");
would become ambiguous.

These added constructors are possible:
  Dialog(const Glib::ustring& title, bool modal, bool use_header_bar);
  Dialog(const Glib::ustring& title, Gtk::Window& parent, bool modal,
         bool use_header_bar);

Are there better ideas?
Comment 2 Simonas Kazlauskas 2014-10-16 17:46:05 UTC
Disclaimer: Started using gtkmm and C++ a few weeks ago.

What’s going to happen when/if more DialogFlags are added? Easiest way I see is to provide something that maps closer to the C API like Dialog(const Glib::ustring& title, Gtk::Window& parent, Gtk::DialogFlags& flags).
Comment 3 Kjell Ahlstedt 2014-10-17 17:32:09 UTC
Created attachment 288775 [details] [review]
patch: Gtk::Dialog: Add constructors with DialogFlags

Is this what you want?
  Dialog(const Glib::ustring& title, DialogFlags flags);
  Dialog(const Glib::ustring& title, Gtk::Window& parent, DialogFlags flags);
Comment 4 Simonas Kazlauskas 2014-10-18 12:59:30 UTC
Something like that, yes. Thanks for taking time to create the patch.
Comment 5 Kjell Ahlstedt 2014-10-19 08:56:18 UTC
Created attachment 288832 [details] [review]
patch: Gtk::Dialog: Add constructors with DialogFlags

Here's a better version of the patch. Same API, added comments in the .ccg
file.

I'll push it when there are enough reasons to add a gtkmm-3-14 branch in
the git repository. There is at least one more reason already,
https://mail.gnome.org/archives/gtkmm-list/2014-October/msg00008.html.
Comment 7 Kjell Ahlstedt 2014-12-13 10:00:17 UTC
Created attachment 292655 [details]
Test case

Calling gtk_dialog_new_with_buttons() in the new Gtk::Dialog constructors is
bad. The created object gets the wrong GType, GtkDialog instead of
gtkmm__GtkDialog. One drawback (probably not the only one) is that overridden
default signal handlers are not called, as shown by the attached test case.

I have pushed this commit, which fixes the constructors:
https://git.gnome.org/browse/gtkmm/commit/?id=f4cc27adecbb70b91b77526235f1192c228a0a18