Skip to content

Commit 7c4ae08

Browse files
authored
feat: select entire URL on double-click (#10132)
## Why When double-clicking on a URL like `https://example.com`, only `https` or `//example.com` gets selected because `:` and `/` are word boundaries. Users expect the entire URL to be selected. ## How Added URL detection to the double-click handler in `Surface.zig`: 1. Before falling back to `selectWord`, try to detect if the clicked position is part of a URL 2. Uses the pre-compiled link regexes from user configuration (same patterns used for cmd+click) 3. If a URL is found at the position, select the entire URL 4. Otherwise, fall back to normal word selection The implementation: - Respects user's link configuration (disabled URLs won't trigger selection) - Reuses pre-compiled regexes from `DerivedConfig.links` (no per-click compilation) - Follows the same patterns as `linkAtPos` ## What - `src/Surface.zig`: Added `urlAtPin()` helper function and modified double-click handler - `src/terminal/StringMap.zig`: Added 2 tests for URL detection following existing test patterns
2 parents 323d362 + 6659315 commit 7c4ae08

File tree

2 files changed

+188
-27
lines changed

2 files changed

+188
-27
lines changed

src/Surface.zig

Lines changed: 60 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ const DerivedConfig = struct {
327327
window_width: u32,
328328
title: ?[:0]const u8,
329329
title_report: bool,
330-
links: []Link,
330+
links: []DerivedConfig.Link,
331331
link_previews: configpkg.LinkPreviews,
332332
scroll_to_bottom: configpkg.Config.ScrollToBottom,
333333
notify_on_command_finish: configpkg.Config.NotifyOnCommandFinish,
@@ -347,7 +347,7 @@ const DerivedConfig = struct {
347347

348348
// Build all of our links
349349
const links = links: {
350-
var links: std.ArrayList(Link) = .empty;
350+
var links: std.ArrayList(DerivedConfig.Link) = .empty;
351351
defer links.deinit(alloc);
352352
for (config.link.links.items) |link| {
353353
var regex = try link.oniRegex();
@@ -1599,10 +1599,10 @@ fn mouseRefreshLinks(
15991599
}
16001600

16011601
const link = (try self.linkAtPos(pos)) orelse break :link .{ null, false };
1602-
switch (link[0]) {
1602+
switch (link.action) {
16031603
.open => {
16041604
const str = try self.io.terminal.screens.active.selectionString(alloc, .{
1605-
.sel = link[1],
1605+
.sel = link.selection,
16061606
.trim = false,
16071607
});
16081608
break :link .{
@@ -1613,7 +1613,7 @@ fn mouseRefreshLinks(
16131613

16141614
._open_osc8 => {
16151615
// Show the URL in the status bar
1616-
const pin = link[1].start();
1616+
const pin = link.selection.start();
16171617
const uri = self.osc8URI(pin) orelse {
16181618
log.warn("failed to get URI for OSC8 hyperlink", .{});
16191619
break :link .{ null, false };
@@ -4141,9 +4141,24 @@ pub fn mouseButtonCallback(
41414141
}
41424142
},
41434143

4144-
// Double click, select the word under our mouse
4144+
// Double click, select the word under our mouse.
4145+
// First try to detect if we're clicking on a URL to select the entire URL.
41454146
2 => {
4146-
const sel_ = self.io.terminal.screens.active.selectWord(pin.*);
4147+
const sel_ = sel: {
4148+
// Try link detection without requiring modifier keys
4149+
if (self.linkAtPin(
4150+
pin.*,
4151+
null,
4152+
)) |result_| {
4153+
if (result_) |result| {
4154+
break :sel result.selection;
4155+
}
4156+
} else |_| {
4157+
// Ignore any errors, likely regex errors.
4158+
}
4159+
4160+
break :sel self.io.terminal.screens.active.selectWord(pin.*);
4161+
};
41474162
if (sel_) |sel| {
41484163
try self.io.terminal.screens.active.select(sel);
41494164
try self.queueRender();
@@ -4331,16 +4346,18 @@ fn clickMoveCursor(self: *Surface, to: terminal.Pin) !void {
43314346
}
43324347
}
43334348

4349+
const Link = struct {
4350+
action: input.Link.Action,
4351+
selection: terminal.Selection,
4352+
};
4353+
43344354
/// Returns the link at the given cursor position, if any.
43354355
///
43364356
/// Requires the renderer mutex is held.
43374357
fn linkAtPos(
43384358
self: *Surface,
43394359
pos: apprt.CursorPos,
4340-
) !?struct {
4341-
input.Link.Action,
4342-
terminal.Selection,
4343-
} {
4360+
) !?Link {
43444361
// Convert our cursor position to a screen point.
43454362
const screen: *terminal.Screen = self.renderer_state.terminal.screens.active;
43464363
const mouse_pin: terminal.Pin = mouse_pin: {
@@ -4361,14 +4378,27 @@ fn linkAtPos(
43614378
const cell = rac.cell;
43624379
if (!cell.hyperlink) break :hyperlink;
43634380
const sel = terminal.Selection.init(mouse_pin, mouse_pin, false);
4364-
return .{ ._open_osc8, sel };
4381+
return .{ .action = ._open_osc8, .selection = sel };
43654382
}
43664383

4367-
// If we have no OSC8 links then we fallback to regex-based URL detection.
4368-
// If we have no configured links we can save a lot of work going forward.
4384+
// Fall back to configured links
4385+
return try self.linkAtPin(mouse_pin, mouse_mods);
4386+
}
4387+
4388+
/// Detects if a link is present at the given pin.
4389+
///
4390+
/// If mouse mods is null then mouse mod requirements are ignored (all
4391+
/// configured links are checked).
4392+
///
4393+
/// Requires the renderer state mutex is held.
4394+
fn linkAtPin(
4395+
self: *Surface,
4396+
mouse_pin: terminal.Pin,
4397+
mouse_mods: ?input.Mods,
4398+
) !?Link {
43694399
if (self.config.links.len == 0) return null;
43704400

4371-
// Get the line we're hovering over.
4401+
const screen: *terminal.Screen = self.renderer_state.terminal.screens.active;
43724402
const line = screen.selectLine(.{
43734403
.pin = mouse_pin,
43744404
.whitespace = null,
@@ -4383,20 +4413,23 @@ fn linkAtPos(
43834413
}));
43844414
defer strmap.deinit(self.alloc);
43854415

4386-
// Go through each link and see if we clicked it
43874416
for (self.config.links) |link| {
4388-
switch (link.highlight) {
4417+
// Skip highlight/mods check when mouse_mods is null (double-click mode)
4418+
if (mouse_mods) |mods| switch (link.highlight) {
43894419
.always, .hover => {},
4390-
.always_mods, .hover_mods => |v| if (!v.equal(mouse_mods)) continue,
4391-
}
4420+
.always_mods, .hover_mods => |v| if (!v.equal(mods)) continue,
4421+
};
43924422

43934423
var it = strmap.searchIterator(link.regex);
43944424
while (true) {
43954425
var match = (try it.next()) orelse break;
43964426
defer match.deinit();
43974427
const sel = match.selection();
43984428
if (!sel.contains(screen, mouse_pin)) continue;
4399-
return .{ link.action, sel };
4429+
return .{
4430+
.action = link.action,
4431+
.selection = sel,
4432+
};
44004433
}
44014434
}
44024435

@@ -4427,11 +4460,11 @@ fn mouseModsWithCapture(self: *Surface, mods: input.Mods) input.Mods {
44274460
///
44284461
/// Requires the renderer state mutex is held.
44294462
fn processLinks(self: *Surface, pos: apprt.CursorPos) !bool {
4430-
const action, const sel = try self.linkAtPos(pos) orelse return false;
4431-
switch (action) {
4463+
const link = try self.linkAtPos(pos) orelse return false;
4464+
switch (link.action) {
44324465
.open => {
44334466
const str = try self.io.terminal.screens.active.selectionString(self.alloc, .{
4434-
.sel = sel,
4467+
.sel = link.selection,
44354468
.trim = false,
44364469
});
44374470
defer self.alloc.free(str);
@@ -4444,7 +4477,7 @@ fn processLinks(self: *Surface, pos: apprt.CursorPos) !bool {
44444477
},
44454478

44464479
._open_osc8 => {
4447-
const uri = self.osc8URI(sel.start()) orelse {
4480+
const uri = self.osc8URI(link.selection.start()) orelse {
44484481
log.warn("failed to get URI for OSC8 hyperlink", .{});
44494482
return false;
44504483
};
@@ -5287,11 +5320,11 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !bool
52875320
self.renderer_state.mutex.lock();
52885321
defer self.renderer_state.mutex.unlock();
52895322
if (try self.linkAtPos(pos)) |link_info| {
5290-
const url_text = switch (link_info[0]) {
5323+
const url_text = switch (link_info.action) {
52915324
.open => url_text: {
52925325
// For regex links, get the text from selection
52935326
break :url_text (self.io.terminal.screens.active.selectionString(self.alloc, .{
5294-
.sel = link_info[1],
5327+
.sel = link_info.selection,
52955328
.trim = self.config.clipboard_trim_trailing_spaces,
52965329
})) catch |err| {
52975330
log.err("error reading url string err={}", .{err});
@@ -5301,7 +5334,7 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !bool
53015334

53025335
._open_osc8 => url_text: {
53035336
// For OSC8 links, get the URI directly from hyperlink data
5304-
const uri = self.osc8URI(link_info[1].start()) orelse {
5337+
const uri = self.osc8URI(link_info.selection.start()) orelse {
53055338
log.warn("failed to get URI for OSC8 hyperlink", .{});
53065339
return false;
53075340
};

src/terminal/StringMap.zig

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,3 +147,131 @@ test "StringMap searchIterator" {
147147

148148
try testing.expect(try it.next() == null);
149149
}
150+
151+
test "StringMap searchIterator URL detection" {
152+
if (comptime !build_options.oniguruma) return error.SkipZigTest;
153+
154+
const testing = std.testing;
155+
const alloc = testing.allocator;
156+
const url = @import("../config/url.zig");
157+
158+
// Initialize URL regex
159+
try oni.testing.ensureInit();
160+
var re = try oni.Regex.init(
161+
url.regex,
162+
.{},
163+
oni.Encoding.utf8,
164+
oni.Syntax.default,
165+
null,
166+
);
167+
defer re.deinit();
168+
169+
// Initialize our screen with text containing a URL
170+
var s = try Screen.init(alloc, .{ .cols = 40, .rows = 5, .max_scrollback = 0 });
171+
defer s.deinit();
172+
try s.testWriteString("hello https://example.com/path world");
173+
174+
// Get the line
175+
const line = s.selectLine(.{
176+
.pin = s.pages.pin(.{ .active = .{
177+
.x = 10,
178+
.y = 0,
179+
} }).?,
180+
}).?;
181+
var map: StringMap = undefined;
182+
const sel_str = try s.selectionString(alloc, .{
183+
.sel = line,
184+
.trim = false,
185+
.map = &map,
186+
});
187+
alloc.free(sel_str);
188+
defer map.deinit(alloc);
189+
190+
// Search for URL match
191+
var it = map.searchIterator(re);
192+
{
193+
var match = (try it.next()).?;
194+
defer match.deinit();
195+
196+
const sel = match.selection();
197+
// URL should start at x=6 ("https://example.com/path" starts after "hello ")
198+
try testing.expectEqual(point.Point{ .screen = .{
199+
.x = 6,
200+
.y = 0,
201+
} }, s.pages.pointFromPin(.screen, sel.start()).?);
202+
// URL should end at x=29 (end of "/path")
203+
try testing.expectEqual(point.Point{ .screen = .{
204+
.x = 29,
205+
.y = 0,
206+
} }, s.pages.pointFromPin(.screen, sel.end()).?);
207+
}
208+
209+
try testing.expect(try it.next() == null);
210+
}
211+
212+
test "StringMap searchIterator URL with click position" {
213+
if (comptime !build_options.oniguruma) return error.SkipZigTest;
214+
215+
const testing = std.testing;
216+
const alloc = testing.allocator;
217+
const url = @import("../config/url.zig");
218+
219+
// Initialize URL regex
220+
try oni.testing.ensureInit();
221+
var re = try oni.Regex.init(
222+
url.regex,
223+
.{},
224+
oni.Encoding.utf8,
225+
oni.Syntax.default,
226+
null,
227+
);
228+
defer re.deinit();
229+
230+
// Initialize our screen with text containing a URL
231+
var s = try Screen.init(alloc, .{ .cols = 40, .rows = 5, .max_scrollback = 0 });
232+
defer s.deinit();
233+
try s.testWriteString("hello https://example.com world");
234+
235+
// Simulate clicking on "example" (x=14)
236+
const click_pin = s.pages.pin(.{ .active = .{
237+
.x = 14,
238+
.y = 0,
239+
} }).?;
240+
241+
// Get the line
242+
const line = s.selectLine(.{
243+
.pin = click_pin,
244+
}).?;
245+
var map: StringMap = undefined;
246+
const sel_str = try s.selectionString(alloc, .{
247+
.sel = line,
248+
.trim = false,
249+
.map = &map,
250+
});
251+
alloc.free(sel_str);
252+
defer map.deinit(alloc);
253+
254+
// Search for URL match and verify click position is within URL
255+
var it = map.searchIterator(re);
256+
var found_url = false;
257+
while (true) {
258+
var match = (try it.next()) orelse break;
259+
defer match.deinit();
260+
261+
const sel = match.selection();
262+
if (sel.contains(&s, click_pin)) {
263+
found_url = true;
264+
// Verify URL bounds
265+
try testing.expectEqual(point.Point{ .screen = .{
266+
.x = 6,
267+
.y = 0,
268+
} }, s.pages.pointFromPin(.screen, sel.start()).?);
269+
try testing.expectEqual(point.Point{ .screen = .{
270+
.x = 24,
271+
.y = 0,
272+
} }, s.pages.pointFromPin(.screen, sel.end()).?);
273+
break;
274+
}
275+
}
276+
try testing.expect(found_url);
277+
}

0 commit comments

Comments
 (0)