Possibly dub: bug 459520

dub=dup=duplicate

bug #459520 is about removing the group, here we want to rename an existing group. It's not a dup.

Created attachment 100211 [details] [review]
proposed patch

Attached patch implements this request using a new GtkDialog popping up when clicking rename in the menu.

Created attachment 100212 [details] [review]
proposed patch v2

Forgot a g_free (), sorry for the spam...this is the right patch.

Not sure we need a popup here. Why not just directly edit in the treeview?

That could be interesting to edit in the treeview, for contacts too. However, I think we can't do that for groups because clicking on the group expand/collapse it.

I'll review the patch this afternoon, thank.

I don't suggest to edit the group name each time we click on it. But you right click -> Renamme and then you can edit in the treeview

Good idea!

Cosimo: What do you think about this proposal? Could you modify your patch to implement that?

Guillaume > AFAIK Cosimo did the patch but he was facing some strange GtkTreeView behaviours.

Yes, I'm working on this, but I have to deal with some unexpected GtkCellRenderer behaviour...the first time I want to rename a group, I have to right-click->rename twice to activate the entry on the treeview. Does not happen if I try to do it again then...very strange.
I'll post my updates here when I have something that works properly.

No progress? If you post the code you have so far maybe some GTK guru will help.

Created attachment 101946 [details] [review]
non-working patch

This is what I've done so far, without luck.
I hope someone could find what we're doing wrong :)

Created attachment 102352 [details] [review]
Add edited-callback to GtkCellRenderer( EmpathyCellRendererText)

Add the pointer "GtkCellRenderer *text_rend" on "struct _EmpathyContactListView".
Add callbacks to signals editing-started, editing-canceled, edited.

It doesn't seems to work here, nothing happens when I right-click on a group and click rename :(

The first time, after the right-click on a group and click rename, you need one left-click.
Then works well.
This code is based of pidgin-code, and pidgin runs same.

Right, pidgin is bugged too... We should try to fix that problem, does the bug comes from GTK itself?

Btw, please try to respect coding style, functions should be like that:

static void
my_function (typ1      *arg1,
             tyyyyype2 *arg2)
{
        foo (x);
}

Do not make lines longer than 80 chars.
In signals cb you can directly set the type of the user_data instead of gpointer.

Thanks.

Also, did you wrote your patch based on Cosimo Cecchi's or did you wrote it from scratch?

Created attachment 102473 [details] [review]
Add edited-callback to GtkCellRenderer( EmpathyCellRendererText)   	 

Correction to coding style.
I wrote my patch from scratch based on pidgin-code, I began to write the patch in December.

BTW, the left-click thing is exactly the same issue you'll get with my patch.
Maybe both we and pidgin guys are doing something wrong, but chances are that the bug is really in GTK+ itself.

@Juan: I think it's better to have the text in the entry updated by writing the implementation of start_editing in the EmpathyCellRendererText class than linking to the start-editing signal, getting the text from the model and set it manually in the entry. Also, once you have it in the class, you won't have to do the steps above each time you want to edit the renderer. You can look or c/p from my patch (nevermind the rewrite of the button-press-event handler, which I did because I thought the problem was there).

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

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

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

Let's wait for the new ContactList API before implementing this.

I have working code for this but it's blocked by bug #669984 and https://bugs.freedesktop.org/show_bug.cgi?id=45982

Created attachment 211373 [details] [review]
add empathy_connection_aggregator_rename_group()

Created attachment 211374 [details] [review]
individual-view: add menu item to rename groups

Created attachment 211375 [details] [review]
live-search: don't forward shift keys

Not doing so cancel any group renaming operation.

Review of attachment 211374 [details] [review]:

::: libempathy-gtk/empathy-individual-view.c
@@ +1830,3 @@
+  g_object_set (priv->text_renderer, "editable", FALSE, NULL);
+
+  if (tp_str_empty (new_name))

could verify if it is only whitespaces too, maybe.

Review of attachment 211374 [details] [review]:

::: libempathy-gtk/empathy-individual-view.c
@@ +1830,3 @@
+  g_object_set (priv->text_renderer, "editable", FALSE, NULL);
+
+  if (tp_str_empty (new_name))

could verify if it is only whitespaces too, maybe.

Review of attachment 211375 [details] [review]:

::: libempathy-gtk/empathy-live-search.c
@@ +365,3 @@
+       event->keyval == GDK_KEY_Shift_R)
+       return FALSE;
+

Does it still work to type capital letters in the live search with this?

Review of attachment 211374 [details] [review]:

::: libempathy-gtk/empathy-individual-view.c
@@ +1830,3 @@
+  g_object_set (priv->text_renderer, "editable", FALSE, NULL);
+
+    gchar *new_name,

Good point. It's worth stripping the string first anyway.

Created attachment 212409 [details] [review]
strip the new group name

(In reply to comment #32)
> Review of attachment 211375 [details] [review]:
> 
> ::: libempathy-gtk/empathy-live-search.c
> @@ +365,3 @@
> +       event->keyval == GDK_KEY_Shift_R)
> +       return FALSE;
> +
> 
> Does it still work to type capital letters in the live search with this?

Yes, that's a separate key event with the "Shift" flag on.

ok, looks good now, +1

Attachment 211373 [details] pushed as 69f4cb2 - add empathy_connection_aggregator_rename_group()
Attachment 211374 [details] pushed as 5405e5f - individual-view: add menu item to rename groups
Attachment 211375 [details] pushed as b5063fd - live-search: don't forward shift keys
Attachment 212409 [details] pushed as 4551d97 - strip the new group name

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

Hi,

I've the feeling that this functionality is gone in 3.6 due to the rewritten rooster.

Should I open a new bug?

(In reply to comment #39)
> Hi,
> 
> I've the feeling that this functionality is gone in 3.6 due to the rewritten
> rooster.
> 
> Should I open a new bug?

Indeed, see bug #679919