Rework control shift press handling - Gtk4 prep

[jeremypw]

Extracted from #780

Clarify how this works by having explicit booleans control-pressed and shift-pressed.

Note: In Gtk4 the work around of explicitly passing events to the key controller is not needed but must be kept here.

The copy-last-output' seems to work the same as in master`

[danirabbit]

It seems like these are only used inside the key press handler. Why are we setting these variables outside of that function at all?

[jeremypw]

Because they need to maintain their state between key presses when multiple keys are held down sequentially.

[danirabbit]

I'm not sure I understand what you mean. Testing locally, replacing all of this with checking for CONTROL and SHIFT mask in modifiers seems to work as expected. Can you provide steps where it doesn't work?

[danirabbit]

Here's what I'm testing

diff --git a/src/Widgets/TerminalWidget.vala b/src/Widgets/TerminalWidget.vala
index cc779d30..eca5e5c4 100644
--- a/src/Widgets/TerminalWidget.vala
+++ b/src/Widgets/TerminalWidget.vala
@@ -148,8 +148,6 @@ namespace Terminal {
         private Gtk.GestureMultiPress primary_gesture;
         private Gtk.GestureMultiPress secondary_gesture;
 
-        private bool control_pressed = false;
-        private bool shift_pressed = false;
         private double scroll_delta = 0.0;
 
         public signal void cwd_changed (string cwd);
@@ -196,7 +194,7 @@ namespace Terminal {
                 propagation_phase = TARGET
             };
             key_controller.key_pressed.connect (key_pressed);
-            key_controller.key_released.connect (key_released);
+            key_controller.key_released.connect (() => scroll_controller.flags = NONE);
             key_controller.focus_out.connect (() => scroll_controller.flags = NONE);
 
             primary_gesture = new Gtk.GestureMultiPress (this) {
@@ -337,13 +335,9 @@ namespace Terminal {
             switch (keyval) {
                 case Gdk.Key.Control_R:
                 case Gdk.Key.Control_L:
-                    scroll_controller.flags = VERTICAL;
-                    control_pressed = true;
-                    break;
                 case Gdk.Key.Shift_R:
                 case Gdk.Key.Shift_L:
                     scroll_controller.flags = VERTICAL;
-                    shift_pressed = true;
                     break;
                 case Gdk.Key.Alt_L:
                 case Gdk.Key.Alt_R:
@@ -381,7 +375,7 @@ namespace Terminal {
                     popup_context_menu (rect);
                     break;
                 default:
-                    if (!(control_pressed || shift_pressed) || !(Gtk.accelerator_get_default_mod_mask () in modifiers)) {
+                    if (!(CONTROL_MASK in modifiers || SHIFT_MASK in modifiers) || !(Gtk.accelerator_get_default_mod_mask () in modifiers)) {
                         remember_command_start_position ();
                     }
                     break;
@@ -403,11 +397,11 @@ namespace Terminal {
                 return false;
             }
 
-            if (control_pressed && Application.settings.get_boolean ("natural-copy-paste")) {
+            if (CONTROL_MASK in modifiers && Application.settings.get_boolean ("natural-copy-paste")) {
                 if (match_keycode (Gdk.Key.c, keycode)) {
                     if (get_has_selection ()) {
                         copy_clipboard ();
-                        if (!shift_pressed) { // Shift not pressed
+                        if (!(SHIFT_MASK in modifiers)) { // Shift not pressed
                             unselect_all ();
                         }
                         return true;
@@ -427,23 +421,6 @@ namespace Terminal {
             return false;
         }
 
-        private void key_released (uint keyval, uint keycode, Gdk.ModifierType modifiers) {
-            switch (keyval) {
-                case Gdk.Key.Control_R:
-                case Gdk.Key.Control_L:
-                    control_pressed = false;
-                    break;
-                case Gdk.Key.Shift_R:
-                case Gdk.Key.Shift_L:
-                    shift_pressed = true;
-                    break;
-                default:
-                    break;
-            }
-
-            scroll_controller.flags = NONE;
-        }
-
         private void setup_menu () {
             // Update the "Paste" menu option
             clipboard.request_targets ((clipboard, atoms) => {

[jeremypw]

This had to be done in the Gtk4 version because

Gtk4 seems to still have a bug in the "state" parameter of the key controller signals in that if multiple modifiers are held down this parameter only includes one of them so we still have to track the state ourselves.

I'll check again whether this fix is needed.

[jeremypw]

Confirmed that the changes made in the Gtk4 branch are not needed here and not worth backporting so closing. Sorry for the confusion.

[jeremypw]

Looks like I misdiagnosed the problem with the Gtk4 port - we do not need to maintain control_pressed and shift_pressed variables there either. But we do have to handle the native mode of copy-paste as well. Just leaving it to the Vte.Terminal like we do here does not work.