Merge pull request #659 from mitchellh/xterm-stuff

xterm audit: REP, XTSHIFTESCAPE

src/Surface.zig

@@ -142,6 +142,7 @@ const DerivedConfig = struct {
     confirm_close_surface: bool,
     mouse_interval: u64,
     mouse_hide_while_typing: bool,
+    mouse_shift_capture: configpkg.MouseShiftCapture,
     macos_non_native_fullscreen: configpkg.NonNativeFullscreen,
     macos_option_as_alt: configpkg.OptionAsAlt,
     window_padding_x: u32,
@@ -162,6 +163,7 @@ const DerivedConfig = struct {
             .confirm_close_surface = config.@"confirm-close-surface",
             .mouse_interval = config.@"click-repeat-interval" * 1_000_000, // 500ms
             .mouse_hide_while_typing = config.@"mouse-hide-while-typing",
+            .mouse_shift_capture = config.@"mouse-shift-capture",
             .macos_non_native_fullscreen = config.@"macos-non-native-fullscreen",
             .macos_option_as_alt = config.@"macos-option-as-alt",
             .window_padding_x = config.@"window-padding-x",
@@ -1501,6 +1503,36 @@ fn mouseReport(
     try self.io_thread.wakeup.notify();
 }
 
+/// Returns true if the shift modifier is allowed to be captured by modifier
+/// events. It is up to the caller to still verify it is a situation in which
+/// shift capture makes sense (i.e. left button, mouse click, etc.)
+fn mouseShiftCapture(self: *const Surface, lock: bool) bool {
+    // Handle our never/always case where we don't need a lock.
+    switch (self.config.mouse_shift_capture) {
+        .never => return false,
+        .always => return true,
+        .false, .true => {},
+    }
+
+    if (lock) self.renderer_state.mutex.lock();
+    defer if (lock) self.renderer_state.mutex.unlock();
+
+    // If thet terminal explicitly requests it then we always allow it
+    // since we processed never/always at this point.
+    switch (self.io.terminal.flags.mouse_shift_capture) {
+        .false => return false,
+        .true => return true,
+        .null => {},
+    }
+
+    // Otherwise, go with the user's preference
+    return switch (self.config.mouse_shift_capture) {
+        .false => false,
+        .true => true,
+        .never, .always => unreachable, // handled earlier
+    };
+}
+
 pub fn mouseButtonCallback(
     self: *Surface,
     action: input.MouseButtonState,
@@ -1519,11 +1551,20 @@ pub fn mouseButtonCallback(
     // Always show the mouse again if it is hidden
     if (self.mouse.hidden) self.showMouse();
 
+    // This is set to true if the terminal is allowed to capture the shift
+    // modifer. Note we can do this more efficiently probably with less
+    // locking/unlocking but clicking isn't that frequent enough to be a
+    // bottleneck.
+    const shift_capture = self.mouseShiftCapture(true);
+
     // Shift-click continues the previous mouse state if we have a selection.
     // cursorPosCallback will also do a mouse report so we don't need to do any
     // of the logic below.
     if (button == .left and action == .press) {
-        if (mods.shift and self.mouse.left_click_count > 0) {
+        if (mods.shift and
+            self.mouse.left_click_count > 0 and
+            !shift_capture)
+        {
             // Checking for selection requires the renderer state mutex which
             // sucks but this should be pretty rare of an event so it won't
             // cause a ton of contention.
@@ -1546,8 +1587,9 @@ pub fn mouseButtonCallback(
         self.renderer_state.mutex.lock();
         defer self.renderer_state.mutex.unlock();
         if (self.io.terminal.flags.mouse_event != .none) report: {
-            // Shift overrides mouse "grabbing" in the window, taken from Kitty.
+            // If we have shift-pressed and we aren't allowed to capture it,
-            if (mods.shift) break :report;
+            // then we do not do a mouse report.
+            if (mods.shift and button == .left and !shift_capture) break :report;
 
             // In any other mouse button scenario without shift pressed we
             // clear the selection since the underlying application can handle
@@ -1682,7 +1724,9 @@ pub fn cursorPosCallback(
     // Do a mouse report
     if (self.io.terminal.flags.mouse_event != .none) report: {
         // Shift overrides mouse "grabbing" in the window, taken from Kitty.
-        if (self.mouse.mods.shift) break :report;
+        if (self.mouse.mods.shift and
+            self.mouse.click_state[@intFromEnum(input.MouseButton.left)] == .press and
+            !self.mouseShiftCapture(false)) break :report;
 
         // We use the first mouse button we find pressed in order to report
         // since the spec (afaict) does not say...

src/config.zig

@@ -6,6 +6,7 @@ pub const Config = @import("config/Config.zig");
 // Field types
 pub const CopyOnSelect = Config.CopyOnSelect;
 pub const Keybinds = Config.Keybinds;
+pub const MouseShiftCapture = Config.MouseShiftCapture;
 pub const NonNativeFullscreen = Config.NonNativeFullscreen;
 pub const OptionAsAlt = Config.OptionAsAlt;
 

src/config/Config.zig

@@ -197,6 +197,28 @@ palette: Palette = .{},
 /// cursor is over the active terminal surface.
 @"mouse-hide-while-typing": bool = false,
 
+/// Determines whether running programs can detect the shift key pressed
+/// with a mouse click. Typically, the shift key is used to extend mouse
+/// selection.
+///
+/// The default value of "false" means that the shift key is not sent
+/// with the mouse protocol and will extend the selection. This value
+/// can be conditionally overridden by the running program with the
+/// XTSHIFTESCAPE sequence.
+///
+/// The value "true" means that the shift key is sent with the mouse
+/// protocol but the running program can override this behavior with
+/// XTSHIFTESCAPE.
+///
+/// The value "never" is the same as "false" but the running program
+/// cannot override this behavior with XTSHIFTESCAPE. The value "always"
+/// is the same as "true" but the running program cannot override this
+/// behavior with XTSHIFTESCAPE.
+///
+/// If you always want shift to extend mouse selection even if the
+/// program requests otherwise, set this to "never".
+@"mouse-shift-capture": MouseShiftCapture = .false,
+
 /// The opacity level (opposite of transparency) of the background.
 /// A value of 1 is fully opaque and a value of 0 is fully transparent.
 /// A value less than 0 or greater than 1 will be clamped to the nearest
@@ -1930,3 +1952,11 @@ pub const GtkSingleInstance = enum {
     false,
     true,
 };
+
+/// See mouse-shift-capture
+pub const MouseShiftCapture = enum {
+    false,
+    true,
+    always,
+    never,
+};

src/terminal/Terminal.zig

@@ -102,6 +102,11 @@ flags: packed struct {
     /// this was called so we have to track it separately.
     mouse_event: MouseEvents = .none,
     mouse_format: MouseFormat = .x10,
+
+    /// Set via the XTSHIFTESCAPE sequence. If true (XTSHIFTESCAPE = 1)
+    /// then we want to capture the shift key for the mouse protocol
+    /// if the configuration allows it.
+    mouse_shift_capture: enum { null, false, true } = .null,
 } = .{},
 
 /// The event types that can be reported for mouse-related activities.
@@ -856,11 +861,10 @@ fn clearWideSpacerHead(self: *Terminal) void {
 }
 
 /// Print the previous printed character a repeated amount of times.
-pub fn printRepeat(self: *Terminal, count: usize) !void {
+pub fn printRepeat(self: *Terminal, count_req: usize) !void {
-    // TODO: test
     if (self.previous_char) |c| {
-        var i: usize = 0;
+        const count = @max(count_req, 1);
-        while (i < count) : (i += 1) try self.print(c);
+        for (0..count) |_| try self.print(c);
     }
 }
 
@@ -6082,3 +6086,47 @@ test "Terminal: tabClear all" {
     try t.horizontalTab();
     try testing.expectEqual(@as(usize, 29), t.screen.cursor.x);
 }
+
+test "Terminal: printRepeat simple" {
+    const alloc = testing.allocator;
+    var t = try init(alloc, 5, 5);
+    defer t.deinit(alloc);
+
+    try t.printString("A");
+    try t.printRepeat(1);
+
+    {
+        var str = try t.plainString(testing.allocator);
+        defer testing.allocator.free(str);
+        try testing.expectEqualStrings("AA", str);
+    }
+}
+
+test "Terminal: printRepeat wrap" {
+    const alloc = testing.allocator;
+    var t = try init(alloc, 5, 5);
+    defer t.deinit(alloc);
+
+    try t.printString("    A");
+    try t.printRepeat(1);
+
+    {
+        var str = try t.plainString(testing.allocator);
+        defer testing.allocator.free(str);
+        try testing.expectEqualStrings("    A\nA", str);
+    }
+}
+
+test "Terminal: printRepeat no previous character" {
+    const alloc = testing.allocator;
+    var t = try init(alloc, 5, 5);
+    defer t.deinit(alloc);
+
+    try t.printRepeat(1);
+
+    {
+        var str = try t.plainString(testing.allocator);
+        defer testing.allocator.free(str);
+        try testing.expectEqualStrings("", str);
+    }
+}

src/terminal/stream.zig

@@ -815,6 +815,30 @@ pub fn Stream(comptime Handler: type) type {
                             }
                         },
 
+                        // XTSHIFTESCAPE
+                        '>' => if (@hasDecl(T, "setMouseShiftCapture")) capture: {
+                            const capture = switch (action.params.len) {
+                                0 => false,
+                                1 => switch (action.params[0]) {
+                                    0 => false,
+                                    1 => true,
+                                    else => {
+                                        log.warn("invalid XTSHIFTESCAPE command: {}", .{action});
+                                        break :capture;
+                                    },
+                                },
+                                else => {
+                                    log.warn("invalid XTSHIFTESCAPE command: {}", .{action});
+                                    break :capture;
+                                },
+                            };
+
+                            try self.handler.setMouseShiftCapture(capture);
+                        } else log.warn(
+                            "unimplemented CSI callback: {}",
+                            .{action},
+                        ),
+
                         else => log.warn(
                             "unknown CSI s with intermediate: {}",
                             .{action},
@@ -1521,3 +1545,26 @@ test "stream: DECSCUSR without space" {
     try s.nextSlice("\x1B[1q");
     try testing.expect(s.handler.style == null);
 }
+
+test "stream: XTSHIFTESCAPE" {
+    const H = struct {
+        escape: ?bool = null,
+
+        pub fn setMouseShiftCapture(self: *@This(), v: bool) !void {
+            self.escape = v;
+        }
+    };
+
+    var s: Stream(H) = .{ .handler = .{} };
+    try s.nextSlice("\x1B[>2s");
+    try testing.expect(s.handler.escape == null);
+
+    try s.nextSlice("\x1B[>s");
+    try testing.expect(s.handler.escape.? == false);
+
+    try s.nextSlice("\x1B[>0s");
+    try testing.expect(s.handler.escape.? == false);
+
+    try s.nextSlice("\x1B[>1s");
+    try testing.expect(s.handler.escape.? == true);
+}

src/termio/Exec.zig

@@ -1521,6 +1521,10 @@ const StreamHandler = struct {
         }
     }
 
+    pub fn setMouseShiftCapture(self: *StreamHandler, v: bool) !void {
+        self.terminal.flags.mouse_shift_capture = if (v) .true else .false;
+    }
+
     pub fn setAttribute(self: *StreamHandler, attr: terminal.Attribute) !void {
         switch (attr) {
             .unknown => |unk| log.warn("unimplemented or unknown SGR attribute: {any}", .{unk}),

website/app/vt/rep/page.mdx

@@ -0,0 +1,53 @@
+import VTSequence from "@/components/VTSequence";
+
+# Repeat Previous Character (REP)
+
+<VTSequence sequence={["CSI", "Pn", "b"]} />
+
+Repeat the previously printed character `n` times.
+
+The parameter `n` must be an integer greater than or equal to 1. If `n` is less than
+or equal to 0, adjust `n` to be 1. If `n` is omitted, `n` defaults to 1.
+
+In xterm, only characters with single byte (less than decimal 256) are
+supported. In most other mainstream terminals, any character is supported.
+
+Each repeated character behaves identically to if it was manually typed in.
+Therefore, soft-wrapping, margins, etc. all behave the same as if the
+character was typed.
+
+The previously printed character is any character that is printed through
+any means. The previously printed character is not limited to characters
+a user manually types. If there is no previously typed character, this sequence
+does nothing.
+
+## Validation
+
+### REP V-1: Simple Usage
+
+```bash
+printf "\033[1;1H" # move to top-left
+printf "\033[0J" # clear screen
+printf "A"
+printf "\033[b"
+```
+
+```
+|AAc_______|
+```
+
+### REP V-2: Soft-Wrap
+
+```bash
+cols=$(tput cols)
+printf "\033[1;1H" # move to top-left
+printf "\033[0J" # clear screen
+printf "\033[${cols}G"
+printf "A"
+printf "\033[b"
+```
+
+```
+|_________A|
+|Ac________|
+```

website/app/vt/xtshiftescape/page.mdx

@@ -0,0 +1,41 @@
+import VTSequence from "@/components/VTSequence";
+
+# Set Shift-Escape (XTSHIFTESCAPE)
+
+<VTSequence sequence={["CSI", ">", "Pn", "s"]} />
+
+Configure whether mouse reports are allowed to capture the `shift` modifier.
+
+The parameter `n` must be an integer equal to 0 or 1. If `n` is omitted,
+`n` defaults to 1. If `n` is an invalid value, this sequence does nothing.
+
+When a terminal program requests [mouse reporting](#TODO), some mouse
+reporting modes also report the modifier keys that are pressed (control, shift,
+etc.). This would disable the ability for a terminal user to natively select
+text if they typically select text using left-click and drag, since the
+left-click event is captured by the running program.
+
+To get around this limitation, many terminal emulators (including xterm)
+use the `shift` modifier to disable mouse reporting temporarily, allowing
+native text selection to work. In this scenario, however, the running
+terminal program cannot detect shift-clicks because the terminal emulator
+captures the event.
+
+This sequence (`XTSHIFTESCAPE`) allows configuring this behavior. If
+`n` is `0`, the terminal is allowed to override the shift key and not pass
+it through to the terminal program. If `n` is `1`, the terminal program
+is requesting that the shift modifier is sent using standard mouse
+reporting formats.
+
+In either case, the terminal emulator is not forced to respect this request.
+For example, `xterm` has a `never` and `always` terminal configuration
+to never allow terminal programs to capture shift or to always allow them,
+respectively. If either of these configurations are set, `XTSHIFTESCAPE`
+has zero effect.
+
+`xterm` also has `false` and `true` terminal configurations. In the `false`
+scenario, the terminal emulator will override `shift` (not allow the terminal
+program to see it) _unless it is explicitly requested_ via `XTSHIFTESCAPE`.
+The `true` scenario is the exact opposite: pass the shift modifier through
+to the running terminal program unless the terminal program explicitly states
+it doesn't need to know about it (`n = 0`).