From 3ac3923e314954451a38a94918552b60ad3a93b3 Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Thu, 1 Oct 2026 08:08:54 -0300 Subject: A Rename the server answers for this file alone, while other open files of its language say the name, is previewed, never applied; Lspwhy shows the files synced first zls renaming at a declaration answers with that file's edits only, cold or warm, whatever it was told of (checked against zls directly: from a call site in main.zig it renames both files, from the declaration in util.zig it never does), so the first Rename applied in util.zig and left main.zig broken. The client now looks for the old name in the other open files of the language; when one says it and the answer left it alone, the edits are a preview with a row naming each such file, and the message row says the server renamed in this file only. Lspwhy carries the open files too, so its trace shows `synced X first` for the query it explains. Co-Authored-By: Claude Opus 5.5 --- src/host_io.zig | 4 ++- src/lsp/lsp_client.zig | 67 ++++++++++++++++++++++++++++++++++++++++++++++---- src/ninep/ctl.zig | 8 ++++++ src/pardes.zig | 10 ++++++-- 4 files changed, 81 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/host_io.zig b/src/host_io.zig index 9e9a37ed..a488f0f8 100644 --- a/src/host_io.zig +++ b/src/host_io.zig @@ -188,7 +188,9 @@ pub const Lsp = struct { }; // ponytail: a copy of every open file per such query; a server // already told of one is sent nothing more (syncDoc). - if (pardes.lsp.reachesOtherFiles(req.kind)) { + // Lspwhy too: the query it explains may be one of those, and its + // trace shows each file synced first. + if (pardes.lsp.reachesOtherFiles(req.kind) or req.kind == .explain) { var others: std.ArrayList(pardes.lsp.Doc) = .empty; errdefer { for (others.items) |doc| { diff --git a/src/lsp/lsp_client.zig b/src/lsp/lsp_client.zig index 338dba61..91899a15 100644 --- a/src/lsp/lsp_client.zig +++ b/src/lsp/lsp_client.zig @@ -503,7 +503,7 @@ fn run(c: *Conn, si: usize, arena: std.mem.Allocator, req: lsp.Req, out: *std.Io try syncDoc(c, si, arena, uri.items, req.source); // Every other open file this server answers for, before a query whose // answer reaches them: zls renames only in the documents it has. - for (req.others) |doc| if (specFor(doc.path) == si and !std.mem.eql(u8, doc.path, req.path)) { + if (lsp.reachesOtherFiles(kind)) for (req.others) |doc| if (specFor(doc.path) == si and !std.mem.eql(u8, doc.path, req.path)) { var other: std.ArrayList(u8) = .empty; try uriOf(&other, arena, doc.path); try syncDoc(c, si, arena, other.items, doc.source); @@ -578,7 +578,14 @@ fn run(c: *Conn, si: usize, arena: std.mem.Allocator, req: lsp.Req, out: *std.Io try app(&b, arena, ",\"newName\":"); try jstr(&b, arena, req.arg); const result = (try call(c, arena, "textDocument/rename", b.items, deadline)) orelse return; - try renameEdits(&cx, c.caps.enc, uri.items, result, req.source); + // Other open files that say the old name, should the server + // answer for this file alone (zls renaming at a declaration does, + // whatever it was told): applied here, they would no longer build. + var also: std.ArrayList([]const u8) = .empty; + const old = identAt(req.source, req.offset); + if (old.len > 0) for (req.others) |doc| if (specFor(doc.path) == si and !std.mem.eql(u8, doc.path, req.path) and holdsWord(doc.source, old)) + also.append(arena, doc.path) catch {}; + try renameEdits(&cx, c.caps.enc, uri.items, result, req.source, old, also.items); }, .format => { var b: std.ArrayList(u8) = .empty; @@ -740,7 +747,33 @@ fn waitFresh(c: *Conn, uri: []const u8, deadline: i64) void { /// multi-file rename, file creates/renames) becomes a PREVIEW — one location /// row per would-be edit, in the same buffer `gr` fills, because silently /// applying a fraction of a workspace rename would be worse than either. -fn renameEdits(cx: *Cx, enc: Enc, self_uri: []const u8, result: std.json.Value, src: []const u8) Err!void { +/// The identifier at byte `at` of `src`: the word a rename there names. +fn identAt(src: []const u8, at: usize) []const u8 { + const isId = struct { + fn f(ch: u8) bool { + return std.ascii.isAlphanumeric(ch) or ch == '_'; + } + }.f; + var start = @min(at, src.len); + while (start > 0 and isId(src[start - 1])) start -= 1; + var end = @min(at, src.len); + while (end < src.len and isId(src[end])) end += 1; + return src[start..end]; +} + +/// Whether `text` holds `word` as a whole identifier. +fn holdsWord(text: []const u8, word: []const u8) bool { + var from: usize = 0; + while (std.mem.indexOfPos(u8, text, from, word)) |i| : (from = i + 1) { + const before = i == 0 or !(std.ascii.isAlphanumeric(text[i - 1]) or text[i - 1] == '_'); + const end = i + word.len; + const after = end == text.len or !(std.ascii.isAlphanumeric(text[end]) or text[end] == '_'); + if (before and after) return true; + } + return false; +} + +fn renameEdits(cx: *Cx, enc: Enc, self_uri: []const u8, result: std.json.Value, src: []const u8, old: []const u8, also: []const []const u8) Err!void { var edits: std.ArrayList(PutEdit) = .empty; var foreign = false; @@ -778,13 +811,20 @@ fn renameEdits(cx: *Cx, enc: Enc, self_uri: []const u8, result: std.json.Value, } }; - if (foreign) { + // Answered for this file alone while other open files say the old + // name: previewed, never applied, each such file named in a row. + const partial = !foreign and also.len > 0; + if (foreign or partial) { // the preview needs the self-file rows too — the point is the full map for (edits.items) |ed| { const lc = lsp.lineCol(src, ed.start); try lsp.row(cx.out, lsp.rel(cx.base, cx.cur_path), lc.line, lc.col, flat(cx.arena, ed.text)); cx.rows += 1; } + if (partial) for (also) |path| { + try cx.out.print("{s}: also says {s}, which the server did not rename\n", .{ lsp.rel(cx.base, path), old }); + cx.rows += 1; + }; return; } sortEdits(edits.items); @@ -2077,7 +2117,7 @@ test "LSP client rename and format propagate incomplete edit encoding" { var out: std.Io.Writer = .fixed(buffer[0..capacity]); var cx: Cx = .{ .arena = arena.allocator(), .base = "/", .cur_path = "/file.c", .cur_src = "abc xyz", .out = &out }; const result = if (rename) - renameEdits(&cx, .utf8, uri, parsed.value, cx.cur_src) + renameEdits(&cx, .utf8, uri, parsed.value, cx.cur_src, "abc", &.{}) else formatEdits(&cx, .utf8, get(get(parsed.value, "changes"), uri).?, cx.cur_src); if (capacity < expected.len) { @@ -2090,6 +2130,23 @@ test "LSP client rename and format propagate incomplete edit encoding" { } } +test "a rename answered for this file alone, while another open file says the old name, previews and names it" { + const gpa = std.testing.allocator; + const parsed = try std.json.parseFromSlice(std.json.Value, gpa, + \\{"changes":{"file:///w/util.zig":[{"range":{"start":{"line":0,"character":7},"end":{"line":0,"character":10}},"newText":"sum"}]}} + , .{}); + defer parsed.deinit(); + var arena: std.heap.ArenaAllocator = .init(gpa); + defer arena.deinit(); + var buffer: [512]u8 = undefined; + var out: std.Io.Writer = .fixed(&buffer); + var cx: Cx = .{ .arena = arena.allocator(), .base = "/w", .cur_path = "/w/util.zig", .cur_src = "pub fn add() void {}", .out = &out }; + try renameEdits(&cx, .utf8, "file:///w/util.zig", parsed.value, cx.cur_src, "add", &.{"/w/main.zig"}); + try std.testing.expectEqualStrings("util.zig:1:8 sum\nmain.zig: also says add, which the server did not rename\n", out.buffered()); + try std.testing.expectEqualStrings("add", identAt("x = add(1);", 6)); + try std.testing.expect(holdsWord("util.add(1)", "add") and !holdsWord("addNumbers", "add")); +} + test "frameNext distinguishes incomplete, valid and poison frames" { try std.testing.expectEqual(FrameStep.incomplete, frameNext("Content-Length: 5\r\n")); try std.testing.expectEqual(FrameStep.incomplete, frameNext("Content-Length: 5\r\n\r\nhel")); diff --git a/src/ninep/ctl.zig b/src/ninep/ctl.zig index 1a2dbdbc..c564da6e 100644 --- a/src/ninep/ctl.zig +++ b/src/ninep/ctl.zig @@ -4253,3 +4253,11 @@ test "a Restore reads a clean file from disk; an unsaved one whose file changed pardes.exec.saveFile(restored, 0); try testing.expect(th.logHas(restored, "modified on disk since read (Save again to overwrite)")); } + +test "a Rename the server answered for this file alone, with other open files saying the name, is previewed and says so" { + const p = try withFile(testing.allocator, "x\n"); + defer p.deinit(); + p.lspRequest(p.active, .rename, "sum"); + p.lspResponse(p.lsp_wait.?.id, "util.zig:1:8 sum\nmain.zig: also says add, which the server did not rename\n"); + try testing.expect(th.logHas(p, "Rename: previewed, not applied: the server renamed in this file only, and 1 other open file(s) say the name")); +} diff --git a/src/pardes.zig b/src/pardes.zig index ca0980f4..b15cfb00 100644 --- a/src/pardes.zig +++ b/src/pardes.zig @@ -6770,8 +6770,14 @@ pub const Pardes = struct { p.fs.lsp_result = rp.serial; }; if (w.kind == .rename) { - var said_buf: [96]u8 = undefined; - p.setMessage(w.pane, std.fmt.bufPrint(&said_buf, "Rename: {d} edit(s) across files, previewed, not applied", .{nrows}) catch "Rename: previewed"); + var said_buf: [128]u8 = undefined; + // Rows that only name a file the server left alone (lsp_client + // renameEdits) are no edits. + const unrenamed = std.mem.count(u8, rows, ", which the server did not rename\n"); + p.setMessage(w.pane, if (unrenamed > 0) + std.fmt.bufPrint(&said_buf, "Rename: previewed, not applied: the server renamed in this file only, and {d} other open file(s) say the name", .{unrenamed}) catch "Rename: previewed" + else + std.fmt.bufPrint(&said_buf, "Rename: {d} edit(s) across files, previewed, not applied", .{nrows}) catch "Rename: previewed"); } if (panes.Output.traits(from).jumps) { const result = p.panes[pane.search_pane orelse return] orelse return; -- cgit v1.3