[geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

List overview All Threads

newer

older

[geany/geany] GTK3: scrolling...

[geany/geany] Saveactions "instant...

Colomban Wendling

21 Feb 2017 21 Feb '17

11:32 p.m.

Since GTK 3.19.12, GtkComboBox has an intermediate GtkBox internal child wrapping the inner GtkEntry. To get the entry, `gtk_bin_get_child()` still works as it is part of the API, but the change breaks the assumption we had that it works the other way around, that `gtk_widget_get_parent(gtk_bin_get_child(combobox)) == combobox`. So, while this assumption seemed reasonable, stop relying on it as it is effectively not correct on GTK 3.20 and newer.

See: https://git.gnome.org/browse/gtk+/commit/?id=222c43fc60362eeb97ce2d5e3a5583a... You can view, comment on, or merge this pull request online at:

https://github.com/geany/geany/pull/1404

-- Commit Summary --

* Fix search history filling on GTK >= 3.20

-- File Changes --

M src/search.c (48)

-- Patch Links --

https://github.com/geany/geany/pull/1404.patch https://github.com/geany/geany/pull/1404.diff

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404

Attachments:

attachment.htm (text/html — 3.0 KB)

Show replies by date

Colomban Wendling

21 Feb 21 Feb

11:34 p.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

@codebrainz @elextr @frlan this should be a fairly straightforward search & replace for the most part, but I'd really like you check the diff and/or test the search dialog to make sure I didn't make a blunt mistake. Seems good to me, and to work find here, but still.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#issuecomment-281504311

Colomban Wendling

11:36 p.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

BTW, without this you can see scary warnings on the CLI (why I noticed this initially): ``` (geany:32379): GLib-GObject-WARNING **: invalid cast from 'GtkBox' to 'GtkComboBoxText'

(geany:32379): GLib-GObject-WARNING **: invalid cast from 'GtkBox' to 'GtkComboBox'

(geany:32379): Gtk-CRITICAL **: gtk_combo_box_get_model: assertion 'GTK_IS_COMBO_BOX (combo_box)' failed

(geany:32379): Gtk-CRITICAL **: gtk_tree_model_get_iter_first: assertion 'GTK_IS_TREE_MODEL (tree_model)' failed

(geany:32379): Gtk-CRITICAL **: gtk_combo_box_text_insert: assertion 'GTK_IS_COMBO_BOX_TEXT (combo_box)' failed

(geany:32379): Gtk-CRITICAL **: gtk_tree_model_get_iter: assertion 'GTK_IS_TREE_MODEL (tree_model)' failed

```

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#issuecomment-281504809

elextr

22 Feb 22 Feb

2:13 a.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

Works for me but then it does without this patch (GTK3.18.9)

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#issuecomment-281535804

Matthew Brush

2:14 a.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

I have the same GTK+ version as @elextr, will look over the diff in a while though.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#issuecomment-281536019

Matthew Brush

2:23 a.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

codebrainz commented on this pull request.

...

@@ -641,27 +643,27 @@ static void create_replace_dialog(void)

label_replace = gtk_label_new_with_mnemonic(_("Replace wit_h:")); gtk_misc_set_alignment(GTK_MISC(label_replace), 0, 0.5);

- entry_find = gtk_combo_box_text_new_with_entry(); - ui_entry_add_clear_icon(GTK_ENTRY(gtk_bin_get_child(GTK_BIN(entry_find)))); - gtk_label_set_mnemonic_widget(GTK_LABEL(label_find), entry_find); - gtk_entry_set_width_chars(GTK_ENTRY(gtk_bin_get_child(GTK_BIN(entry_find))), 50); - ui_hookup_widget(replace_dlg.dialog, entry_find, "entry_find"); - replace_dlg.find_entry = gtk_bin_get_child(GTK_BIN(entry_find)); + replace_dlg.find_combobox = gtk_combo_box_text_new_with_entry(); + replace_dlg.find_entry = gtk_bin_get_child(GTK_BIN(replace_dlg.find_combobox));

Isn't this still relying on internal child packing which is not part of the "public" API? For example next minor version they could decide it looks better with a frame around the entry and break it again, right?

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#pullrequestreview-23116844

Matthew Brush

2:26 a.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

Looks OK by a quick inspection, aside from the one question I asked.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#issuecomment-281538144

elextr

2:31 a.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

elextr commented on this pull request.

...

@@ -641,27 +643,27 @@ static void create_replace_dialog(void)

label_replace = gtk_label_new_with_mnemonic(_("Replace wit_h:")); gtk_misc_set_alignment(GTK_MISC(label_replace), 0, 0.5);

Wouldn't that change the CSS, which is part of the published API?

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#discussion_r102364479

Matthew Brush

2:33 a.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

codebrainz commented on this pull request.

...

@@ -641,27 +643,27 @@ static void create_replace_dialog(void)

label_replace = gtk_label_new_with_mnemonic(_("Replace wit_h:")); gtk_misc_set_alignment(GTK_MISC(label_replace), 0, 0.5);

Ah, it seems to be [documented](https://developer.gnome.org/gtk3/stable/GtkComboBoxText.html):

...

The entry itself can be accessed by calling gtk_bin_get_child() on the combo box.

Though having a `gtk_combo_box_text_get_entry()` function would inspire more confidence that weren't going to break it.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#discussion_r102364759

Matthew Brush

3:17 a.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

codebrainz commented on this pull request.

...

@@ -641,27 +643,27 @@ static void create_replace_dialog(void)

label_replace = gtk_label_new_with_mnemonic(_("Replace wit_h:")); gtk_misc_set_alignment(GTK_MISC(label_replace), 0, 0.5);

@elextr I think the only "promise" that's made is that functions, structs, enums, etc won't change incompatibly (except for theme engines apparently), when it comes to CSS I don't think any promises are made (see many many rants online about theme devs hating on GTK+ for this).

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#discussion_r102369886

Colomban Wendling

9:20 a.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

b4n commented on this pull request.

...

@@ -641,27 +643,27 @@ static void create_replace_dialog(void)

label_replace = gtk_label_new_with_mnemonic(_("Replace wit_h:")); gtk_misc_set_alignment(GTK_MISC(label_replace), 0, 0.5);

@codebrainz as you found out yourself in the docs, it's documented to work. And yeah, having a dedicated accessor would seem better, but well.

...

Wouldn't that change the CSS, which is part of the published API?

@elextr Bah, they changed a lot of CSS stuff in 3.20, so I guess their answer is that it's not stable or something.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#discussion_r102404515

gmc-holle

9:30 a.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

I suffered from this issue in Geany 1.29 that the search (and replace) combo boxes did not anymore.

Thanks for this patch! It works great again and - that's the important thing - I can use them again and I need them quite a lot ;)

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#issuecomment-281602363

Colomban Wendling

9:31 a.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

@gmc-holle great, thanks for testing!

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#issuecomment-281602605

elextr

10:08 a.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

elextr commented on this pull request.

...

@@ -641,27 +643,27 @@ static void create_replace_dialog(void)

label_replace = gtk_label_new_with_mnemonic(_("Replace wit_h:")); gtk_misc_set_alignment(GTK_MISC(label_replace), 0, 0.5);

@b4n, remember "stable" isn't until 3.22 ;-)

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#discussion_r102412461

Colomban Wendling

27 Feb 27 Feb

2:23 p.m.

New subject: [geany/geany] Fix search history filling on GTK >= 3.20 (#1404)

Merged #1404.

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1404#event-978325051

2856

Age (days ago)

2862

Last active (days ago)

github-comments@lists.geany.org

14 comments

4 participants

tags (0)

participants (4)

Colomban Wendling
elextr
gmc-holle
Matthew Brush