Created attachment 49337 [details] [review]
v1: Initial patch based on EelEditableLabel

TODO: popup, blinking cursor, investigate ::selectable/::editable relation.

Created attachment 77086 [details] [review]
Updated patch to apply against HEAD with blinking cursor

I updated the patch to apply cleanly to HEAD (gtk+ 2.11), and added cursor blinking, based on the implementation in gtkentry.c

Created attachment 77259 [details] [review]
Fix text_deletion seg fault

The previous patch would cause applications to crash if set_text was used. This patch fixes the error.

I'm interested in using this in an application. Is there any way this is going to get into GTK+ 2.12?

just a first rough look at the patch: it breaks the ABI so you'll need to remove as much padding pointers as needed by the new signals. unfortunately, GtkLabel has just 4 padding slots and the patch adds 5 new signals.

instead of adding editability to the label widget, can't we create a GtkEditableLabel subclass which implements the GtkEditable interface? this would avoid breaking the ABI.

I agree. That would leave the 4 padding signal slots, and wouldn't require the GtkLabel widget to carry around any extra baggage. Given that the summer has just started, I'm going to implement a GtkEditableLabel. Is anyone else already doing this, or think it should be another way, or something of the sort? I don't want to reinvent the wheel.

Formally rejecting the patch. ABI needs to be preserved.

I am working on a patch which does what Emmanuele proposed in comment 6.

Created attachment 118336 [details] [review]
implement GtkEditableLabel as subclass of GtkLabel

Implementation of GtkEditableLabel as a subclass of GtkLabel. Works fine in my testing, comments more than welcome!

(In reply to comment #10)
> Created an attachment (id=118336) [edit]
> implement GtkEditableLabel as subclass of GtkLabel
> 
> Implementation of GtkEditableLabel as a subclass of GtkLabel. Works fine in my
> testing, comments more than welcome!
> 

A very quick look at it, and it looks quite acceptable.
However, I'm a bit worried about the size of changes in GtkLabel itself. Renaming and & making a bit of the private api accessible from GtkEditableLabel is fine.
I'm more worried about the other changes. Can they be removed/refactored to reduce the likelihood of regressions?
[ I didn't look at the content of the changes, just talking about the presence of them per se ]

(In reply to comment #11)

> A very quick look at it, and it looks quite acceptable.
> However, I'm a bit worried about the size of changes in GtkLabel itself.
> Renaming and & making a bit of the private api accessible from GtkEditableLabel
> is fine.
> I'm more worried about the other changes. Can they be removed/refactored to
> reduce the likelihood of regressions?
> [ I didn't look at the content of the changes, just talking about the presence
> of them per se ]

Hi Johan,

If you exclude renaming and opening up some bits of private API, the main change to GtkLabel is text managing (i.e. implementing insert_text () and delete_text () into GtkLabel and use these functions to mimic gtk_label_set_text () old behavior). AFAICS, this is quite unavoidable as GtkEditableLabel needs a way to insert/delete text into an arbitrary position for implementing GtkEditable and I can't find a way to move insert_text () and delete_text () into GtkEditableLabel without duplicating lots of code and doing ugly tricks.

One question: if I need to add a field (e.g. a guint) to GtkLabel, is it better to add it GSEALed to the GtkLabel struct in gtklabel.h or add it in GtkLabelPrivate and provide getters and setters in gtklabelprivate.h?

Link to Gnome wiki: http://live.gnome.org/ProjectRidley/EelEditableLabel

(In reply to comment #12)
> One question: if I need to add a field (e.g. a guint) to GtkLabel, is it better
> to add it GSEALed to the GtkLabel struct in gtklabel.h or add it in
> GtkLabelPrivate and provide getters and setters in gtklabelprivate.h?

I'd would say that you can simply add it to GtkLabelPrivate struct. As far as I know, in the 2.90 branch, all variables in the public structure will be moved to the private ones, so in GtkLabel.h you will have:

struct _GtkLabel
{
  GtkMisc misc;

  /*< private >*/
  GtkLabelPrivate *priv;
};

Any progress on this?

Seems that this is the only request to get rid of libeel deprecated library (see http://live.gnome.org/ProjectRidley)

so: EditableLabel would be good for 2.x - but it fell off the radar. for 2.90 we can break API/ABI, and add support for editability directly to GtkLabel. we should probably decide this either on the mailing list or in an IRC meeting.

definitely 3.0 material, though.

Recent irc discussion of this contains my thoughts on it

<ebassi> editable label: sub-class or not?
<ebassi> the sub-class idea was due to the "we can't break ABI and we're out of slots on LabelClass"
<ebassi> I guess now we can simply extend that
<mclasen> I must admit I never fully grokked the idea of an editable label as a widget
<mclasen> isn't that just a notebook with a label and an entry ?
<mitch> haha i agree
<mitch> it's a WTF
<ebassi> well, nautilus has them to rename files :-)
<ebassi> it's a bit ad hoc, though
<mitch> ebassi: aren't these tree rows?
<ebassi> no
<ebassi> in icon view
<ebassi> not in list view
* mclasen has done a lot of label + editable widget in a notebook in the accountsdialog
<jjardon> It's needed to replace EelEditableLabel: Its one of the items of ProjectRidley: http://live.gnome.org/ProjectRidley
<mclasen> it can be tricky sometimes to get focus handling right
<jjardon> bug here: https://bugzilla.gnome.org/show_bug.cgi?id=310693
<ebassi> jjardon: yep, EelEditableLabel is what nautilus is using (in tree)
<ebassi> dunno if there are other projects using it, though
<mclasen> I would invite anybody to study the accountsdialog set of label-that-turns-into-entry, label-that=turns-into-button, label-that-turns-into-combo
<jjardon> cosimoc offered to make a patch if an implementation was decided here
<jjardon> mclasen, yeah It's pretty cool ;)
<mclasen> so, I'd be more interested in investigating if there is a more general pattern of editable-uneditable widget that we can  support, instead of just copying eeleditablelabel
<garnacho> I wonder if it doesn't make more sense to make it the other way around, a non-editable entry that looks like a label
<mclasen> the thing is that label and entry have distinct feature sets
<mclasen> there's a big overlap, certainly
<mclasen> but there is also a difference
<garnacho> I see, should check the use cases
<mclasen> labels do urls, they can or cannot be selectable, they can wrap,..
<mclasen> entries can be invisible, they can show icons, they can not wrap, etc etc
<garnacho> a GtkLabel subclass makes sense then
<mclasen> what will that class do ? reimplement gtkentry ?
<ebassi> and in case of word wrapping, does it become a text area? :-)

Created attachment 166759 [details] [review]
another approach

Hi,

I found some time to work on this again, and I came up with this patch, which uses another approach than my last one.

- The implementation is done inside GtkLabel, by implementing the GtkEditable interface.
- Also, I added two methods, gtk_label_[get/set]_editable, and an 'editable' property that do what their name suggest.

The patch reuses most of the code from EelEditableLabel, and it's working fine; I added some test code to testgtk.c to demonstrate the feature. Also, note that this is just the outcome of two afternoons of hacking at GUADEC, so the patch could probably be improved.

Feedback welcome.

Editable label are very useful in some special case. As a member of the openshot team, i would be very happy to use them to rename track, put label on markers and for our title editor for example.

Any progress on this ? I know it's too late for GTK3 inclusion, maybe 3.2 ?

We're moving to gitlab! As part of this move, we are closing bugs that haven't seen activity in more than 5 years. If this issue is still imporant to you and
still relevant with GTK+ 3.22 or master, please consider creating a gitlab issue
for it.

Would still be useful.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gtk/issues/250.