After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769890 - Terminal does not make URIs clickable
Terminal does not make URIs clickable
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: general
3.20.x
Other Linux
: Normal normal
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-08-14 12:53 UTC by Philip Withnall
Modified: 2016-12-24 11:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
opens web links on <CTRL> + 'Mouse Primary Button' click (1.31 KB, patch)
2016-11-20 13:44 UTC, Anoop Chandu
reviewed Details | Review
terminal: add ctrl+click to open URIs (2.41 KB, patch)
2016-11-20 21:30 UTC, Christian Hergert
committed Details | Review
terminal: add menu entries to open and copy links (6.09 KB, patch)
2016-12-22 11:09 UTC, Anoop Chandu
none Details | Review
terminal: add menu entries to open and copy links (5.15 KB, patch)
2016-12-22 17:39 UTC, Anoop Chandu
committed Details | Review

Description Philip Withnall 2016-08-14 12:53:54 UTC
If I'm looking at `git log` in a terminal page, I can't ctrl+click the links (for bug reports, for example) to open them, which is a bit of a pain. Instead, I have to double-click the link to select most of it (the URI scheme and leading colon aren't selected initially) and manually copy into a browser.
Comment 1 Christian Hergert 2016-08-15 11:03:39 UTC
We probably need the regex stuff that gnome-terminal does.
Comment 2 Anoop Chandu 2016-11-20 13:44:02 UTC
Created attachment 340352 [details] [review]
opens web links on <CTRL> + 'Mouse Primary Button' click

As of now this patch detects links like 'http://xxx', 'https://yyy', 'ftp://zzz' only. Links will be opened in default browser by clicking <CTRL> + 'Mouse Primary Button'. 

Can we make popup menu with 'open link' and 'copy link address' options, like in gnome-terminal, when mouse secondary button is clicked on that link?
Comment 3 Christian Hergert 2016-11-20 21:29:39 UTC
Review of attachment 340352 [details] [review]:

::: plugins/terminal/gb-terminal.c
@@ -151,1 +151,5 @@
     }
+  else if ((button->type == GDK_BUTTON_PRESS) && (button->button == GDK_BUTTON_PRIMARY)
+            && ((button->state & GDK_CONTROL_MASK) == GDK_CONTROL_MASK))
+    {
+      gchar *pattern;

pattern is leaked

@@ +230,3 @@
   egg_widget_action_group_attach (self, "terminal");
+
+  gchar *pattern;

const gchar *pattern = "((http|https|ftp)://)\\S+";

Also, declarations need to be at the top of scope (so above the egg_widget_ function call.

@@ +231,3 @@
+
+  gchar *pattern;
+  g_autoptr(GRegex) *g_regex;

Always assign g_autoptr() variables to NULL
Comment 4 Christian Hergert 2016-11-20 21:30:10 UTC
Created attachment 340376 [details] [review]
terminal: add ctrl+click to open URIs

This adds support for opening links in the terminal views by using
ctrl+click similar to gnome-terminal.
Comment 5 Christian Hergert 2016-11-20 21:33:01 UTC
Thanks! I've cleaned up the patch to include the various review fixes.
Since there was not a git commit log for the patch, I just used your
bugzilla information for git author information.

Attachment 340376 [details] pushed as cdf885d - terminal: add ctrl+click to open URIs
Comment 6 Christian Hergert 2016-11-20 21:34:46 UTC
As for adding a menu, the way we do that in Builder is to exted the plugins/terminal/gtk/menus.ui file to include a menu accelerator to activate a GAction. Then add a gaction to the terminal widget which is only "enabled" when the link has been hovered.

Then, when you right click on the url, you'll see the menu item active.
Comment 7 Anoop Chandu 2016-12-22 11:09:14 UTC
Created attachment 342379 [details] [review]
terminal: add menu entries to open and copy links

Added GActions and corresponding menu items which are only enabled and visible when right clicked on some URI.

@hergertme can you explain what menu accel you are referring to?
Comment 8 sébastien lafargue 2016-12-22 12:16:26 UTC
Review of attachment 342379 [details] [review]:

about the accelerators, it was probably about the _ in the menu string that you have added.

Globally, the patch works but need work.

::: plugins/terminal/gb-terminal.c
@@ +59,3 @@
   SELECT_ALL,
+  OPEN_LINK,
+  COPY_LINK_ADDRESS,

better ordering this in alphabetical order

@@ +84,3 @@
+
+  g_free (terminal->url);
+  terminal->url = NULL;

better use g_clear_pointer (&terminal->url, g_free);

@@ +110,3 @@
 
+      terminal->url  = vte_terminal_match_check_event (VTE_TERMINAL (terminal), (GdkEvent *)popup_info->event, NULL);
+      have_url = (terminal->url == NULL) ? FALSE : TRUE;

very verbose, i'll have used directly (terminal->url == NULL) in the egg_widget_action_group_set_action_enabled

or at least if you prefer more clarity just: have_url = (terminal->url == NULL);

@@ +209,3 @@
+{
+  g_assert (GB_IS_TERMINAL (self));
+  g_assert (self->url != NULL);

better use g_assert (!ide_str_empty0 (self->url));

@@ +219,3 @@
+{
+  g_assert (GB_IS_TERMINAL (self));
+  g_assert (self->url != NULL);

better use g_assert (!ide_str_empty0 (self->url));

@@ +275,3 @@
+                  G_TYPE_NONE,
+                  0);
+

not sure of the usefulness of these action signals but at least, as they are dependent of a possible non-existent link under the cursor, they should return a boolean
Comment 9 Anoop Chandu 2016-12-22 17:39:54 UTC
Created attachment 342394 [details] [review]
terminal: add menu entries to open and copy links

Made fixes. Instead of 2 actions for copy and open only action is created with boolean argument.
Comment 10 sébastien lafargue 2016-12-24 11:10:27 UTC
i have changed the patch to keep the two signals, better finally.
You missed the the activation of the action only when the regex match

thanks for the patch.
Comment 11 sébastien lafargue 2016-12-24 11:11:51 UTC
Attachment 342394 [details] pushed as cbe4739 - terminal: add menu entries to open and copy links