GNOME Bugzilla – Bug 769890
Terminal does not make URIs clickable
Last modified: 2016-12-24 11:11: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.
We probably need the regex stuff that gnome-terminal does.
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?
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
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.
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
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.
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?
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
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.
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.
Attachment 342394 [details] pushed as cbe4739 - terminal: add menu entries to open and copy links