macos: show copy menu item if selection start is outside viewport (#8288)

Fixes #8187

This properly handles the scenario with our `read_text` C API when the
selection start is outside the viewport. Previously we'd report null (no
text) which would cascade into things like the right click menu not
showing a copy option.

macos/Sources/Ghostty/SurfaceView_AppKit.swift

@@ -1327,7 +1327,7 @@ extension Ghostty {
             var item: NSMenuItem
 
             // If we have a selection, add copy
-            if self.selectedRange().length > 0 {
+            if let text = self.accessibilitySelectedText(), text.count > 0 {
                 menu.addItem(withTitle: "Copy", action: #selector(copy(_:)), keyEquivalent: "")
             }
             menu.addItem(withTitle: "Paste", action: #selector(paste(_:)), keyEquivalent: "")

src/Surface.zig

@@ -1531,11 +1531,6 @@ pub const Text = struct {
 
     /// The viewport information about this text, if it is visible in
     /// the viewport.
-    ///
-    /// NOTE(mitchellh): This will only be non-null currently if the entirety
-    /// of the selection is contained within the viewport. We don't have a
-    /// use case currently for partial bounds but we should support this
-    /// eventually.
     viewport: ?Viewport = null,
 
     pub const Viewport = struct {
@@ -1546,6 +1541,13 @@ pub const Text = struct {
         /// The linear offset of the start of the selection and the length.
         /// This is "linear" in the sense that it is the offset in the
         /// flattened viewport as a single array of text.
+        ///
+        /// Note: these values are currently wrong if there is a partially
+        /// visible selection in the viewport (i.e. the top-left or
+        /// bottom-right of the selection is outside the viewport). But the
+        /// apprt usecase we have right now doesn't require these to be
+        /// correct so... let's fix this later. The wrong values will always
+        /// be within the text bounds so we aren't risking an overflow.
         offset_start: u32,
         offset_len: u32,
     };
@@ -1587,17 +1589,57 @@ pub fn dumpTextLocked(
 
     // Calculate our viewport info if we can.
     const vp: ?Text.Viewport = viewport: {
-        // If our tl or br is not in the viewport then we don't
+        // If our bottom right pin is before the viewport, then we can't
-        // have a viewport. One day we should extend this to support
+        // possibly have this text be within the viewport.
-        // partial selections that are in the viewport.
+        const vp_tl_pin = self.io.terminal.screen.pages.getTopLeft(.viewport);
-        const tl_pt = self.io.terminal.screen.pages.pointFromPin(
+        const br_pin = sel.bottomRight(&self.io.terminal.screen);
+        if (br_pin.before(vp_tl_pin)) break :viewport null;
+
+        // If our top-left pin is after the viewport, then we can't possibly
+        // have this text be within the viewport.
+        const vp_br_pin = self.io.terminal.screen.pages.getBottomRight(.viewport) orelse {
+            // I don't think this is possible but I don't want to crash on
+            // that assertion so let's just break out...
+            log.warn("viewport bottom-right pin not found, bug?", .{});
+            break :viewport null;
+        };
+        const tl_pin = sel.topLeft(&self.io.terminal.screen);
+        if (vp_br_pin.before(tl_pin)) break :viewport null;
+
+        // We established that our top-left somewhere before the viewport
+        // bottom-right and that our bottom-right is somewhere after
+        // the top-left. This means that at least some portion of our
+        // selection is within the viewport.
+
+        // Our top-left point. If it doesn't exist in the viewport it must
+        // be before and we can return (0,0).
+        const tl_pt: terminal.Point = self.io.terminal.screen.pages.pointFromPin(
             .viewport,
-            sel.topLeft(&self.io.terminal.screen),
+            tl_pin,
-        ) orelse break :viewport null;
+        ) orelse tl: {
+            if (comptime std.debug.runtime_safety) {
+                assert(tl_pin.before(vp_tl_pin));
+            }
+
+            break :tl .{ .viewport = .{} };
+        };
+
+        // Our bottom-right point. If it doesn't exist in the viewport
+        // it must be the bottom-right of the viewport.
         const br_pt = self.io.terminal.screen.pages.pointFromPin(
             .viewport,
-            sel.bottomRight(&self.io.terminal.screen),
+            br_pin,
-        ) orelse break :viewport null;
+        ) orelse br: {
+            if (comptime std.debug.runtime_safety) {
+                assert(vp_br_pin.before(br_pin));
+            }
+
+            break :br self.io.terminal.screen.pages.pointFromPin(
+                .viewport,
+                vp_br_pin,
+            ).?;
+        };
+
         const tl_coord = tl_pt.coord();
         const br_coord = br_pt.coord();
 
@@ -1668,73 +1710,6 @@ pub fn selectionString(self: *Surface, alloc: Allocator) !?[:0]const u8 {
     });
 }
 
-/// Return the apprt selection metadata used by apprt's for implementing
-/// things like contextual information on right click and so on.
-///
-/// This only returns non-null if the selection is fully contained within
-/// the viewport. The use case for this function at the time of authoring
-/// it is for apprt's to implement right-click contextual menus and
-/// those only make sense for selections fully contained within the
-/// viewport. We don't handle the case where you right click a word-wrapped
-/// word at the end of the viewport yet.
-pub fn selectionInfo(self: *const Surface) ?apprt.Selection {
-    self.renderer_state.mutex.lock();
-    defer self.renderer_state.mutex.unlock();
-    const sel = self.io.terminal.screen.selection orelse return null;
-
-    // Get the TL/BR pins for the selection and convert to viewport.
-    const tl = sel.topLeft(&self.io.terminal.screen);
-    const br = sel.bottomRight(&self.io.terminal.screen);
-    const tl_pt = self.io.terminal.screen.pages.pointFromPin(.viewport, tl) orelse return null;
-    const br_pt = self.io.terminal.screen.pages.pointFromPin(.viewport, br) orelse return null;
-    const tl_coord = tl_pt.coord();
-    const br_coord = br_pt.coord();
-
-    // Utilize viewport sizing to convert to offsets
-    const start = tl_coord.y * self.io.terminal.screen.pages.cols + tl_coord.x;
-    const end = br_coord.y * self.io.terminal.screen.pages.cols + br_coord.x;
-
-    // Our sizes are all scaled so we need to send the unscaled values back.
-    const content_scale = self.rt_surface.getContentScale() catch .{ .x = 1, .y = 1 };
-
-    const x: f64 = x: {
-        // Simple x * cell width gives the left
-        var x: f64 = @floatFromInt(tl_coord.x * self.size.cell.width);
-
-        // Add padding
-        x += @floatFromInt(self.size.padding.left);
-
-        // Scale
-        x /= content_scale.x;
-
-        break :x x;
-    };
-
-    const y: f64 = y: {
-        // Simple y * cell height gives the top
-        var y: f64 = @floatFromInt(tl_coord.y * self.size.cell.height);
-
-        // We want the text baseline
-        y += @floatFromInt(self.size.cell.height);
-        y -= @floatFromInt(self.font_metrics.cell_baseline);
-
-        // Add padding
-        y += @floatFromInt(self.size.padding.top);
-
-        // Scale
-        y /= content_scale.y;
-
-        break :y y;
-    };
-
-    return .{
-        .tl_x_px = x,
-        .tl_y_px = y,
-        .offset_start = start,
-        .offset_len = end - start,
-    };
-}
-
 /// Returns the pwd of the terminal, if any. This is always copied because
 /// the pwd can change at any point from termio. If we are calling from the IO
 /// thread you should just check the terminal directly.