Yes, gtkmm has:
http://library.gnome.org/devel/gtkmm/stable/classGtk_1_1ComboBoxEntry.html
and we have GtkComboBoxEntryText too:
http://library.gnome.org/devel/gtkmm/stable/classGtk_1_1ComboBoxEntryText.html

We even have a near-useless text-only TextView just so that complete-beginners don't get too scared:
http://library.gnome.org/devel/gtkmm/stable/classGtk_1_1ListViewText.html

I wonder, is it worth retaining the virtual method to override _get_active_text?

No, that wouldn't be necessary or useful.

Created attachment 157164 [details] [review]
Implement GtkComboBoxText

GtkComboBoxText subclass, mostly untested.

Review of attachment 157164 [details] [review]:

::: gtk/gtkcomboboxtext.h
@@ +51,3 @@
+struct _GtkComboBoxText
+{
+  GtkComboBox parent;

Maybe is a good idea use a "priv" var here for future use

Created attachment 161461 [details] [review]
Implement GtkComboBoxText

Updated patch against current master with some minor corrections

Created attachment 161462 [details] [review]
Use the new API in internal code

Created attachment 161465 [details] [review]
Implement GtkComboBoxText.v2

Sorry, here the correct patch

Created attachment 161476 [details] [review]
Use the new API in internal code.v2

Fixed some compilation warnings

As we going to deprecate api, this should land in 2.22

2.22 has been released. Change target milestore to 2.24

Shall we put this in GTK+ 3?

(In reply to comment #12)
> Shall we put this in GTK+ 3?

If we can get this in 2.24 (to deprecate the "text" convenience API), It'll be available in 3.0 too ;)
I'm updating the patches rigth now

Created attachment 171582 [details] [review]
Implement GtkComboBoxText.v3

Updated patch against current master

Created attachment 171583 [details] [review]
Use the new API in internal code.v3

Updated patch against current master

Created attachment 171585 [details] [review]
Use the new API in internal code.v4

Totally replace all the uses of the old API

Review of attachment 171582 [details] [review]:

::: gtk/gtkcomboboxtext.c
@@ +63,3 @@
+  GtkWidget *combo_box;
+
+  combo_box = g_object_new (GTK_TYPE_COMBO_BOX_TEXT, NULL);

Avoid the local variable here. Just "return g_object_new..." is fine

@@ +156,3 @@
+ */
+void
+gtk_combo_box_text_remove_text (GtkComboBoxText *combo_box,

All the other _text() functions take a const char *text argument, this one takes a gint position. Maybe it should be called just remove(), not remove_text() ?

Created attachment 171652 [details] [review]
Implement GtkComboBoxText.v4

Updated patch with your comments

Review of attachment 171652 [details] [review]:

> +  g_return_if_fail (GTK_IS_COMBO_BOX_TEXT (combo_box));
> +  g_return_if_fail (position >= 0);
> +  g_return_if_fail (text != NULL);
> +
> +  store = GTK_LIST_STORE (gtk_combo_box_get_model (GTK_COMBO_BOX (combo_box)));

The functions should g_return_if_fail (store != NULL); like the current functions do.

Review of attachment 171585 [details] [review]:

::: gtk/gtkcombobox.c
@@ +5277,3 @@
  * #GtkComboBoxEntry<!-- -->s.
  *
  * Returns: a newly allocated string containing the currently active text.

This makes no sense. The existing API doesn't change and the new class would fail if you tried to use it with the wrong widget.

Bah, this is the proper except I commented on:

> @@ -5273,7 +5273,7 @@ gtk_combo_box_remove_text (GtkComboBox *combo_box,
>   *
>   * Returns the currently active string in @combo_box or %NULL if none
>   * is selected.  Note that you can only use this function with combo
> - * boxes constructed with gtk_combo_box_new_text() and with
> + * boxes constructed with #GtkComboBoxText<!-- -->s and with
>   * #GtkComboBoxEntry<!-- -->s.

We've now landed the patch to deprecate/remove comboboxentry and instead add a construct-only has-entry property to combobox. So these patches probably need minor adjustment for that.

Created attachment 172417 [details] [review]
Implement GtkComboBoxText.v5

Updated patch to apply cleanly and added missing return_if_fail to ensure there is a model and column 0 is a string column.

Created attachment 172418 [details] [review]
Use text column value in GtkComboBoxText

For master this goes on top of the previous patch, to make use of the next text column.

Created attachment 172422 [details] [review]
Use the new API in internal code.v5

Updated patch to master, removing the mention of convenience functions from gtk_combo_box_get_active_text() and removing the snippet about the convenience API since we shouldn't recommend it anymore.

Review of attachment 172417 [details] [review]:

Looks good, thanks.

Review of attachment 172418 [details] [review]:

Looks like gtk_combo_box_get_entry_text_column() returns -1 if a column hasn't been explicitly set. 
Do we want to require people to set the column, or default to 0 ?

Review of attachment 172422 [details] [review]:

::: gtk/gtkcombobox.c
@@ +73,3 @@
  * #GtkCellLayout interface. The tree model holding the valid choices is
  * not restricted to a flat list, it can be a real tree, and the popup will
  * reflect the tree structure.

Should maybe add a reference to the comboboxtext here, instead of just removing the paragraph without replacement.

Thanks, in addition to these patches, we need

- GtkComboBoxText added to the docs

- old text api deprecated (we'll remove it after the next release)

I'll look at getting these things done before the release I want to do tonight

I think there should be some clearer way of getting the text entered in the entry, other than calling gtk_bin_get_child(), casting to GtkEntry, and getting its text. This won't necessarily be a value in the drop-down so we can't just get the active iter.

Previously, gtk_combo_box_get_active_text() had that extra purpose, but it has now been deprecated:
http://library.gnome.org/devel/gtk/stable/GtkComboBox.html#gtk-combo-box-get-active-text

Created attachment 172576 [details] [review]
get the text entered in the entry

Shouldn't gtk_combo_box_get_active_text() stay in GtkComboBox, since it also returns the text in the entry? I think it's a bit ugly to internally create an entry and not expose its text as a function.

Created attachment 172583 [details] [review]
Undeprecate gtk_combo_box_get_active_text

Note I also fixed a small bug in gtk_combo_box_get_active_text not using the text column. If we don't agree on this, this should be fix separately.

Created attachment 172584 [details] [review]
Undeprecate gtk_combo_box_get_active_text #2

Including gtk.symbols update.

I'd rather see this split into two functions as it should have been originally. So we could keep the existing function in GtkComboBoxText and add a gtk_combo_box_get_entry_text(). Actually, a set_entry_text() would be helpful too.

Out of curiosity, how is that useful? Not that this really matters, but in my own use in application code I don't think I ever needed to distinguish between the entry text and what the user has selected from the list.

(In reply to comment #36)
>  I don't think I ever needed to distinguish between
> the entry text and what the user has selected from the list.

You can enter text into a GtkComboBoxEntry that is not in the list. I think that's the whole point of a GtkComboBoxEntry - it just provides some pre-selected choices, but you aren't restricted to them.

Yes. My point is,  if in doubt you want the text in the entry, so get_active_text should give you that, or otherwise use the model.

Yes, but you are using the same function for two not-really-related purposes. Leaving it in the base class just for the entry leaves you with still one function that should only be called (unless it has an entry) on the derived class. Which is what this bug was about fixing.

I don't think there's anything bad about using gtk_bin_get_child + entry api.

However, there is some more cleanup we can do here, now that the entry is in GtkComboBox itself. The get_active_text vfunc is no longer needed and can go away (once GtkComboBoxEntry is gone altogether).

We support text columns of any type that can be transformed to string nowadays, so the column type check should go away, and instead the function must use gvalue transformation as needed (see gtk_combo_box_entry_active_changed())

We should probably even document gtk_bin_get_child + entry api as a valid way of doing things.

(In reply to comment #41)
> We should probably even document gtk_bin_get_child + entry api as a valid way
> of doing things.

But that makes the internal arrangement then part of the public API, making it impossible to change that in future. For other widgets we have getter functions.

(In reply to comment #40)
> However, there is some more cleanup we can do here, now that the entry is in
> GtkComboBox itself. The get_active_text vfunc is no longer needed and can go
> away (once GtkComboBoxEntry is gone altogether).

I think get_active_text vfunc is still useful, as subclasses can still implement their own child widget and even if there is an entry it may not be a direct child.
Arguably these classes can and currently do have their own functions, but I think it would be nicer to have support in GtkComboBox proper.

I noticed that the gtk_combo_box_get_active_text() documentation had been changed in the 2.24 branch, appearing to change the behaviour, which would be a break instead of just a deprecation.

So I put that documentation back:
http://git.gnome.org/browse/gtk+/commit/?h=gtk-2-24&id=36a1730bb8e18e91d2e5af0c0be796bad68b8f0d

I also improved the deprecation warning. I hope that seeing the ugliness of it convinces you that GtkComboBox does need some real API to set/get the text of the entry. We already have API for it in gtkmm.

And I just fixed my label->entry typo in that commit.

*** Bug 636174 has been marked as a duplicate of this bug. ***

Created attachment 175620 [details] [review]
Patch to add some documentation for the entry

Javier told me to use this bug even if I think it is a bit unrelated:

For me, it was quite difficult to find out that the label can be accessed using gtk_bin_get_child (GTK_BIN(combo)) and that you can query the text in the entry there.

Attached a patch that adds at least some documentation for it. Feel free to correct the wording as I don't like it too much but I am not a native speaker.

Created attachment 176365 [details] [review]
GtkComboBox(Text): Add documentation about the entry

I second that, and I'd even go further: I think the docs should be much more explicit about the distinction between standard GtkComboBox, GtkComboBox with entry, GtkComboBoxText and GtkComboBoxText with entry. People are migrating from GTK+2 and are likely to wonder "what should I replace my GtkComboBoxEntry with?" I have to admit it took me some time to understand that 1) GtkComboBoxText has no real relation to GtkComboBoxEntry and 2) GtkComboBox:has-entry can be used for that.

See the attached patch for a suggestion.

I also wonder if it wouldn't be better to provide gtk_combo_box_get_entry() to remove the need for calling gtk_bin_get_child(), which always feels like a hack to me.

If we don't do that though, it would be nice to have gtk_combo_box_get/set_entry_text(): most people don't need to access the entry, just to get its contents. And having them in GtkComboBoxText only is weird: if a GtkComboBox has an entry, we can get the text from it, no need for it to be a GtkComboBoxText. In my programs, I sometimes need to set a complex model, and still need to get the contents of the entry easily.

Review of attachment 176365 [details] [review]:

Looks good apart from my one comment.

::: gtk/gtkcomboboxtext.c
@@ +344,3 @@
+ * Returns the currently active string in @combo_box, or %NULL if none
+ * is selected. If @combo_box contains an entry, this function will return
+ * its contents (which can or cannot match an item in the list).

"which can or cannot" is meaningless.

I would change it to:
"If @combo_box contains an entry, this function will return its contents, which will not necessarily be an item from the list."
though I don't like that much more.

When creating a GtkComboboxText with gtkbuilder, one needs to specify manually that the "entry-text-column" property is 0, because this is set in gtk_combo_box_text_new(), which is of course not called by gtkbuilder (probably the same happens with bindings). This needs to be done somehow else (reinstalling the property? in the constructor?) so that this works out of the box.

Retitling for whats left here.

I wish you would more seriously our wish for a real API for getting the Entry.

Created attachment 179521 [details] [review]
Override the default value of some GtkComboBoxText properties

This is cleaner than setting the value in the _new() functions and
makes binding and GtkBuilder instantiation easier.

This is not needed:

  -                                                      GTK_PARAM_READWRITE | G_PARAM_CONSTRUCT));
  +                                                      GTK_PARAM_READWRITE));

Matthias? Are we getting this in before 3.0?

Matthias? Are we getting this in before 3.0?

Review of attachment 179521 [details] [review]:

::: gtk/gtkcomboboxtext.c
@@ +133,1 @@
   return object;

Not sure I fully understand this. Why is the text column only initialized conditionally, but the id-column always ?

Review of attachment 179521 [details] [review]:

::: gtk/gtkcomboboxtext.c
@@ +133,1 @@
   return object;

Because in the case when there is no entry, the text column is already set above (gtk_cell_layout_set_atributes (..., "text", 0, ...)), so it's not necessary to do it again.. or at least that's how I remember it.

Review of attachment 179521 [details] [review]:

::: gtk/gtkcomboboxtext.c
@@ +133,1 @@
   return object;

Because in the case when there is no entry, the text column is already set above (gtk_cell_layout_set_atributes (..., "text", 0, ...)), so it's not necessary to do it again.. or at least that's how I remember it.

I just hit this issue myself, would help a lot of newbie programmers like myself if the default was 0 instead of -1

Can we do this, please? The GtkBuilder problem in comment #50 is very real.

This use of the entry-text-column column is a completely internal detail of GtkComboBoxText. It's documented as only being relevant for comboboxes with entries, but it's necessary for GtkComboBoxText in general. Asking people to set this default in glade files is a horrible hack.

(In reply to comment #59)
> Review of attachment 179521 [details] [review]:
> 
> ::: gtk/gtkcomboboxtext.c
> @@ +133,1 @@
>    return object;
> 
> Because in the case when there is no entry, the text column is already set
> above (gtk_cell_layout_set_atributes (..., "text", 0, ...)), so it's not
> necessary to do it again.. or at least that's how I remember it.

I don't think that is the case. It used to be set to 0 by explicitly passing that property to g_object_new in gtk_combo_box_text_new, but your patch takes that out. 

I think you need to set entry-text-column unconditionally in the constructor.

gtk_cell_layout_set_attributes (..., "text", 0, ...) tells the cell renderer
from which column in the tree model it shall read the text to print. It does
not set GtkComboBox's entry-text-column property, which must be 0 e.g. when
gtk_combo_box_text_get_active_text() is called.

Matthias is right in thinking that the entry-text-column property must be set
unconditionally. The name entry-text-column is slightly misleading here. In a
GtkComboBoxText entry-text-column is used even if has-entry is FALSE.

entry-text-column _is_ set unconditionally, because the patch in comment 53
also adds code to gtk_combo_box_text_class_init(), where added calls to
g_object_class_install_property() set the flag G_PARAM_CONSTRUCT and redefine
the default values of both entry-text-column and id-column.

Isn't it unnecessary to set entry-text-column = 0 and id-column = 1 both in
gtk_combo_box_text_class_init() (with g_object_class_install_property()) and in
gtk_combo_box_text_constructor()?

(In reply to comment #62)
> (In reply to comment #59)
> > Review of attachment 179521 [details] [review] [details]:
> > 
> > ::: gtk/gtkcomboboxtext.c
> > @@ +133,1 @@
> >    return object;
> > 
> > Because in the case when there is no entry, the text column is already set
> > above (gtk_cell_layout_set_atributes (..., "text", 0, ...)), so it's not
> > necessary to do it again.. or at least that's how I remember it.
> 
> I don't think that is the case. It used to be set to 0 by explicitly passing
> that property to g_object_new in gtk_combo_box_text_new, but your patch takes
> that out. 
> 
> I think you need to set entry-text-column unconditionally in the constructor.

I tend to forget stuff after so long, so please someone feel free to pick up on the patch and update/fix it.

Created attachment 204289 [details] [review]
patch: comboboxtext: Set entry-text-column and id-column props in the ctor

What about this patch, based on the patch in comment 53?

entry-text-column and id-column are set unconditionally in the constructor.
There is no redefinition of these properties with
g_object_class_install_property().

The patch also adds a sentence to the description of GtkComboBoxText. A similar
sentence was added to Gtk::ComboBoxText in gtkmm as a fix of bug 619656.
And it deletes a reference to the non-existent function
gtk_combo_box_entry_new_with_text().

Review of attachment 204289 [details] [review]:

Looks ok to me now.

Pushed the patch in comment 65 to master branch.
http://git.gnome.org/browse/gtk+/commit/?id=6aeab7b7ccfcdeb8f871c2744d5fbae24e3bda5f

*** Bug 669121 has been marked as a duplicate of this bug. ***

Mathias, do you want to have this in the gtk-3-2 branch too?

*** Bug 649663 has been marked as a duplicate of this bug. ***