This is not as trivial as I though. Maybe GTK+ could offer us more facilities to do that?

I have the code to add markup to a string. Do I have to attach it so you can use it?

I'm patching it now. it works with channel topics, I still need to find the label of contact information

No way, it works only for simple links, but if you have a "&" or another strange char it complains that you have to escape it.
Unfortunately if I escape it, also the :// are escaped, so I need a smarter way to append markup.
Help appreciated

Created attachment 138803 [details] [review]
url_in_label.patch

this is the patch. I don't know if I can do better, I don't know well how link handling works in GtkLabel. for instance this does not work with youtube videos

Created attachment 138809 [details] [review]
url_in_label commit

I think I managed to do this! (I borrowed the XML function g_markup_escape_text).
Please check this

Created attachment 138810 [details] [review]
url_in_label commit 2

there was an indentation problem, sorry

You should squash the two patches together.

Created attachment 138812 [details] [review]
url_in_label.patch

single patch (but no commitdiff).
I added other checks

Created attachment 138814 [details] [review]
url_in_label.patch

that was git diff --color, sorry

I quickly tested the patch and at first it didn't recognize the URL (it was
between parenthesis), I think it should probably use the same regex as for
detecting URLs in chat views (see empathy-ui-utils.c,
empathy_uri_regex_dup_singleton).

Also it's not useful to have the URL clickable in the contact tooltip, as it's
not possible to click on it.

And last, it didn't make the URL clickable in the contact list (but that is
certainly harder and it was not requested in this bug report).

thanks for pointing out the url regex (I need to learn how to use it)

the contact tooltip uses the same label as the one shown in the contact information dialog, that's why it is shows as a link.

I'll have a look and if I can find where is the status label in the contact list I will try to expand the patch

Nicolò, I experienced a segfault today, while running with your patch applied; this is because empathy_add_markup() got called with a NULL parameter, probably because empathy_contact_get_status() returned that.  (it was in the context of a subscription request).

To know if you are creating a widget for the tooltip, you can check against (information->flags & EMPATHY_CONTACT_WIDGET_FOR_TOOLTIP).

I've got the correct patch now, but I will attach it as soon as I manage to see if the label in the contact list can be markupped too

Is it really a problem if the label in the tooltip has a link inside?

Created attachment 138945 [details] [review]
url_in_label.patch

This works for me.
It does not add the feature directly in the contact list yet, because I don't know how to work wit GtkTreeView. Sorry

Created attachment 138971 [details] [review]
url_in_label.patch

I removed the link in the tooltip using the suggested condition.

I'm stupid, I mixed up 2 branches, sorry...
There are some lines belonging to another branch

Created attachment 138973 [details] [review]
url_in_label.patch

this should be ok

Can someone review it?

GTK 2.18 is required for this markup. I think it's better to wait for Empathy 2.29 to commit this patch, or at least GTK 2.18 stable release.

Yes, you are right, I forgot to say it, sorry.
Let's not forget about this bug.

FWIW GNOME 2.28 is planned to go out with GTK+ 2.18, and it is perfectly okay for the release team to depend on unstable GTK+ 2.17.

Perhaps you could check for the GTK+ version and only call gtk_label_set_markup when available?

(In reply to comment #11)
> And last, it didn't make the URL clickable in the contact list (but that is
> certainly harder and it was not requested in this bug report).

...and is a terrible idea. I'd be incredibly annoyed if I single-clicked the contact list to foreground it and a browser popped up in my face, just as I am when I single-click the contact list and a call window pops up in my face.

(In reply to comment #23)
> ...and is a terrible idea. I'd be incredibly annoyed if I single-clicked the
> contact list to foreground it and a browser popped up in my face, just as I am
> when I single-click the contact list and a call window pops up in my face.

For the record, I also think more items to click on in the contact list is a terribly poor idea.

hi again, not gtk 2.18 has been released, so you can push this patch to git

indeed. my first comment is that the path should bump required version of gtk.

Created attachment 143846 [details] [review]
bump_gtk_2_18.patch

Is this enough? (I'm not expert)

Seems pretty good. Some comments:

 - Coding style, blocks are indented like that:

if (foo)
  {
    baaaaaaaaar (looooon thing more than 80char,
        arg2);
  }
else
  {
    bar ();
  }

Note that it is only for files tha are already in that coding style. Some are still using imendio coding style which is indented using 1 tab.

 - empathy_replace_with_br() is static, we usually name private function without the empathy_ prefix. Should be theme_adium_replace_with_br() to be consistent. Same for empathy_add_smiley_markup().

 - An optimisation I made in theme_adium_parse_body() is to not dup the string if there is no URL match or smiley is not used or there is no '\n'. I think empathy_add_*_markup() and empathy_replace_with_br() should return NULL if the string is unchanged.

 - Please add gtkdoc for empathy_add_link_markup(). I know most functions don't have doc, but we are trying to add that sloooowly :)

The rest looks fine to me. Thanks!

- coding style: in the example I can't understand if I have to use tabs or not, and I didn't understand the amounts of tabs I have to use. Since in my previous patch I used to put the wrapped argument exactly below the '( ' I now put it before, to respect the 80 char wrap. (also because it depends on the amount of spaces in which the editor expands the tab).

- I renamed those 2 static functions.

- the now theme_adium_add_smiley_markup always dups the string during the parse of images. I have no time to rethink the logic (also because it was not written by me). Anyway I prepared the logic when using the function as if it could return NULL, so the only thing you have to do is fix the function body itself.

- simple gtkdoc done

*NOT TESTED IT YET, SO NOT ATTACHING IT*

Created attachment 143987 [details] [review]
url.patch

this patch has a nasty infinite loop bug when someone puts links starting with '('

I don't know how to debug this, can you help?

URLs in a contact's status are still not clickable in 2.27.92 (I've tested) so I'd like to up the version number on this bug.

This won't be fixed in 2.28.x, it is for 2.29

Also as long as the patch has problems, it won't be committed...

I'm now working for a new patch. I hope I'll be able to fix this!

Created attachment 146278 [details] [review]
new patch

This patch works better, and also lets having both an URL and a SMILEY on the same text.

Unfortunately there seems to be a bug in the gtk function I'm trying to use:
gtk_label_set_markup (label, markup_text);

everything works if markup text is for example
<a href="www.gnome.org>www.gnome.org</a>

but if I add ( and ) around, the GUI locks and the cpu starts to spin up.
The problem is not in my code, I'm sure because it works correctly in adium themes.

can you tell me how can I execute gdb to debug this problem?

Note that I made a branch that fix parsing of string in adium:

http://git.collabora.co.uk/?p=user/xclaesse/empathy.git;a=shortlog;h=refs/heads/escape

I think you should rebase your patch on top of that. Hopefully that will get into master soon. See bug #599640

Ok, but I'll do it when you commit to master, so I don't have to rebase my patch lots of times.
Please tell me when!

Created attachment 146286 [details]
backtrace

I caught the backtrace hitting ctrl-c after having attacheced gdb.

do you understad where is the infinite loop?

As Xavier said, let's fix bug #599640 first.

Review of attachment 146278 [details] [review]:

Reject the patch for now

I made a branch using new API:

http://git.collabora.co.uk/?p=user/xclaesse/empathy.git;a=shortlog;h=refs/heads/clickable-link

Created attachment 155143 [details] [review]
diff

Attaching diff for easier review

Review of attachment 155143 [details] [review]:

Looks pretty good to me. Only few details:

::: libempathy-gtk/empathy-contact-widget.c
@@ +953,3 @@
+
+  status = empathy_contact_get_status (information->contact);
+  if (!(information->flags & EMPATHY_CONTACT_WIDGET_FOR_TOOLTIP))

What happen if you display the link in tooltips as well? They won't be clickable but maybe it's worth doing it if only to easily spot the URL.

::: libempathy-gtk/empathy-string-parser.c
@@ +193,3 @@
+
+gchar *
+empathy_add_link_markup (const gchar *text)

Please add a small comment documenting this function.

@@ +195,3 @@
+empathy_add_link_markup (const gchar *text)
+{
+	static EmpathyStringParser parsers[] = {

Why having it static?

Fixed and pushed. Thanks :)

reopening, this is causing a serious regression. That's a GTK issue, see bug #612066.

I reverted the changes until it gets fixed in GTK.

Note: To re-enable, simply revert the revert commit efb42513359f4db545e3312e3c583d07c6306418 :D

Set this bug back to resolved, Guillaume opened a new bug to enable clickable links once GTK bug is fixed: bug #612185