lanoxx / tilda

A Gtk based drop down terminal for Linux and Unix
GNU General Public License v2.0
1.28k stars 161 forks source link

Use match_check_event instead of deprecated match_check #284

Closed egmontkob closed 6 years ago

egmontkob commented 7 years ago

As of VTE version 0.44, vte_terminal_match_check() is deprecated, vte_terminal_match_check_event() should be used instead. The latter one is available since VTE version 0.38, that is, VTE library version 2.91.

VTE 0.44 implements "super smooth scrolling", that is, with the touchpad the contents can be scrolled back pixel by pixel, rather than row by row. vte_terminal_match_check() by design cannot cope with this situation and potentially examines the wrong line (off by one). Using vte_terminal_match_check_event() eliminates this problem.

lanoxx commented 7 years ago

We should definitely do that, but I cant say when I will have time to come up with a patch.

egmontkob commented 7 years ago

Should be as easy as

--- a/src/tilda_terminal.c
+++ b/src/tilda_terminal.c
@@ -982,6 +982,11 @@ static int button_press_cb (G_GNUC_UNUSED GtkWidget *widget, GdkEventButton *eve
             terminal  = VTE_TERMINAL(tt->vte_term);
             ypad = gtk_widget_get_margin_bottom(GTK_WIDGET(terminal));

+#if VTE_CHECK_VERSION(0, 38, 0)
+            match = vte_terminal_match_check_event (terminal,
+                                                    event,
+                                                    &tag);
+#else
             glong column = (glong) ((event->x - ypad) /
                                     vte_terminal_get_char_width (terminal));
             glong row = (glong) ((event->y - ypad) /
@@ -991,6 +996,7 @@ static int button_press_cb (G_GNUC_UNUSED GtkWidget *widget, GdkEventButton *eve
                                               column,
                                               row,
                                               &tag);
+#endif

             /* Check if we can launch a web browser, and do so if possible */
             if (match != NULL)
egmontkob commented 7 years ago

You can move the ypad = ... to the #else branch too.

Also, I've just realized it's broken to subtract the bottom padding from event->x and event->y in the old branch; you should subtract the left and the top padding, respectively.

(On a slightly related note, I don't see any point keeping support for VTE library version 2.90. It should be okay to drop it in favor of 2.91 which has been available in all distros for about two years.)

lanoxx commented 6 years ago

I guess we could remove the old VTE code after the next release.

lanoxx commented 6 years ago

Fixed in: d7a8744e2789c9abefbebcd964c2dea2d0f10e9d

egmontkob commented 6 years ago

vte-0.38 (library version 2.91) is 3 years old. All vte-based apps I'm aware of and supported library version 2.90 have now switched to 2.91. At least Fedora and Debian have already dropped their 2.90 (vte-0.36.x) packages in favor of 2.91.

It's your call of course, but I don't see any rationale keeping 2.90 support around any longer.