From 3e5048ada6945256b9a584324cc461e3bb62d166 Mon Sep 17 00:00:00 2001 From: Masaaki Hirotsu Date: Sat, 3 Oct 2026 08:58:52 +0900 Subject: [PATCH 1/3] fix(lsp): convert positions between byte columns and client offset encoding LSP requests sent the cursor's byte column as the position character and used the response characters as byte columns, so on lines with Japanese text or emoji a UTF-16 server received the wrong position and popups opened on the wrong column. - Convert the request position into each client's offset_encoding and convert every returned range, including document symbols, back to byte columns using the loaded buffer or the file on disk. - Keep all internal locations in byte columns so the same-position filter compares the cursor and results in one unit. - Cover UTF-8 / UTF-16 / UTF-32, multiple clients, unloaded files and requests from a copy popup. --- lua/peekstack/core/location.lua | 102 +++++++++++++ lua/peekstack/providers/lsp.lua | 6 +- lua/peekstack/types.lua | 4 +- tests/lsp_offset_encoding_spec.lua | 236 +++++++++++++++++++++++++++++ 4 files changed, 343 insertions(+), 5 deletions(-) create mode 100644 tests/lsp_offset_encoding_spec.lua diff --git a/lua/peekstack/core/location.lua b/lua/peekstack/core/location.lua index 60a977b..d83acad 100644 --- a/lua/peekstack/core/location.lua +++ b/lua/peekstack/core/location.lua @@ -224,6 +224,108 @@ function M.list_from_lsp(result, provider) return items end +---@param uri string +---@return integer? +local function loaded_buf_for_uri(uri) + for _, bufnr in ipairs(vim.api.nvim_list_bufs()) do + if vim.api.nvim_buf_is_loaded(bufnr) and vim.uri_from_bufnr(bufnr) == uri then + return bufnr + end + end + return nil +end + +---Return a reader for the 0-based lines of a document, preferring the loaded +---buffer (which the LSP server sees) over the file on disk. +---@param uri string +---@return fun(row: integer): string? +local function line_reader(uri) + local bufnr = loaded_buf_for_uri(uri) + if bufnr then + return function(row) + return vim.api.nvim_buf_get_lines(bufnr, row, row + 1, false)[1] + end + end + + local lines + return function(row) + if not lines then + lines = {} + local fname = fs.uri_to_fname(uri) + local file = fname and io.open(fname, "rb") + if file then + for line in file:lines() do + lines[#lines + 1] = line + end + file:close() + end + end + return lines[row + 1] + end +end + +---Byte and LSP columns only differ on lines containing non-ASCII bytes. +---@param line string? +---@return boolean +local function is_multibyte(line) + return line ~= nil and line:find("[\128-\255]") ~= nil +end + +---@param read_line fun(row: integer): string? +---@param pos { line: integer, character: integer } +---@param encoding string +---@return { line: integer, character: integer } +local function position_to_byte(read_line, pos, encoding) + local line = read_line(pos.line) + if not is_multibyte(line) then + return { line = pos.line, character = pos.character } + end + return { line = pos.line, character = vim.str_byteindex(line, encoding, pos.character, false) } +end + +---Convert a byte-column position in a loaded buffer into the column unit of +---an LSP client's offset encoding. +---@param bufnr integer +---@param pos { line: integer, character: integer } 0-based line, byte column +---@param encoding? string client offset encoding (defaults to "utf-16") +---@return { line: integer, character: integer } +function M.position_to_lsp(bufnr, pos, encoding) + encoding = encoding or "utf-16" + if encoding == "utf-8" then + return { line = pos.line, character = pos.character } + end + local line = vim.api.nvim_buf_get_lines(bufnr, pos.line, pos.line + 1, false)[1] + if not is_multibyte(line) then + return { line = pos.line, character = pos.character } + end + return { line = pos.line, character = vim.str_utfindex(line, encoding, pos.character, false) } +end + +---Convert location ranges reported in an LSP client's offset encoding into +---byte columns, the unit used for every internal location. +---@param locations PeekstackLocation[] +---@param encoding? string client offset encoding (defaults to "utf-16") +function M.ranges_to_byte(locations, encoding) + encoding = encoding or "utf-16" + if encoding == "utf-8" then + return + end + ---@type table + local readers = {} + for _, loc in ipairs(locations) do + local reader = readers[loc.uri] + if not reader then + reader = line_reader(loc.uri) + readers[loc.uri] = reader + end + local range = normalize_range(loc.range) + loc.range = { + start = position_to_byte(reader, range.start, encoding), + ["end"] = position_to_byte(reader, range["end"], encoding), + } + end +end + ---@param diags table[]? ---@param provider? string ---@return PeekstackLocation[] diff --git a/lua/peekstack/providers/lsp.lua b/lua/peekstack/providers/lsp.lua index d8bf616..255ad0e 100644 --- a/lua/peekstack/providers/lsp.lua +++ b/lua/peekstack/providers/lsp.lua @@ -133,10 +133,7 @@ local function request(ctx, method, provider, params_modifier, result_mapper, cb for _, client in ipairs(clients) do local params = { textDocument = vim.lsp.util.make_text_document_params(bufnr), - position = { - line = ctx.position.line, - character = ctx.position.character, - }, + position = location.position_to_lsp(bufnr, ctx.position, client.offset_encoding), } if params_modifier then params_modifier(params) @@ -148,6 +145,7 @@ local function request(ctx, method, provider, params_modifier, result_mapper, cb if not err and result then local ok, locs = pcall(mapper, result, provider, ctx) if ok and type(locs) == "table" then + location.ranges_to_byte(locs, client.offset_encoding) vim.list_extend(all_locations, locs) end end diff --git a/lua/peekstack/types.lua b/lua/peekstack/types.lua index 42eeb87..1266d35 100644 --- a/lua/peekstack/types.lua +++ b/lua/peekstack/types.lua @@ -1,4 +1,6 @@ -- Type definitions for Peekstack +---0-based lines and 0-based byte columns. LSP providers convert from and to +---each client's offset encoding at the request boundary. ---@class PeekstackRange ---@field start { line: integer, character: integer } ---@field ["end"] { line: integer, character: integer } @@ -156,7 +158,7 @@ ---@field popup_id integer? ---@field buffer_mode "copy"|"source"|nil ---@field line_offset integer ----@field position { line: integer, character: integer } +---@field position { line: integer, character: integer } 0-based line and byte column in `bufnr` ---@field root_winid integer ---@field from_popup boolean diff --git a/tests/lsp_offset_encoding_spec.lua b/tests/lsp_offset_encoding_spec.lua new file mode 100644 index 0000000..bf6fc2b --- /dev/null +++ b/tests/lsp_offset_encoding_spec.lua @@ -0,0 +1,236 @@ +local peekstack = require("peekstack") +local context = require("peekstack.core.context") +local stack = require("peekstack.core.stack") +local lsp_provider = require("peekstack.providers.lsp") + +-- "target" starts at byte 10 / UTF-16 4 / UTF-32 4. +local CJK_LINE = "日本語 target" +-- "target" starts at byte 5 / UTF-16 3 / UTF-32 2. +local EMOJI_LINE = "😀 target" + +describe("lsp offset encoding", function() + local original_get_clients + local tmpdir + local fname + local uri + + ---@param encoding string + ---@param response? fun(params: table): any + ---@param sent? table[] + ---@return table + local function make_client(encoding, response, sent) + return { + offset_encoding = encoding, + request = function(_, _method, params, handler, _bufnr) + if sent then + table.insert(sent, { encoding = encoding, params = params }) + end + handler(nil, response and response(params) or nil) + end, + } + end + + ---@param line integer + ---@param character integer + ---@param target_uri? string + ---@return table + local function lsp_location(line, character, target_uri) + return { + uri = target_uri or uri, + range = { + start = { line = line, character = character }, + ["end"] = { line = line, character = character + 6 }, + }, + } + end + + ---@param line integer + ---@param col integer + ---@return PeekstackProviderContext + local function ctx_at(line, col) + vim.api.nvim_win_set_cursor(0, { line + 1, col }) + return context.current() + end + + ---@param provider fun(ctx: PeekstackProviderContext, cb: fun(locations: PeekstackLocation[])) + ---@param ctx PeekstackProviderContext + ---@return PeekstackLocation[] + local function collect(provider, ctx) + local received + provider(ctx, function(locations) + received = locations + end) + assert.is_table(received) + return received + end + + before_each(function() + original_get_clients = vim.lsp.get_clients + stack._reset() + peekstack.setup({}) + tmpdir = vim.fn.tempname() + vim.fn.mkdir(tmpdir, "p") + fname = tmpdir .. "/source.txt" + vim.fn.writefile({ CJK_LINE, EMOJI_LINE, "plain target" }, fname) + vim.cmd.edit(vim.fn.fnameescape(fname)) + uri = vim.uri_from_bufnr(0) + end) + + after_each(function() + vim.lsp.get_clients = original_get_clients + local s = stack.current_stack() + for i = #s.popups, 1, -1 do + stack.close(s.popups[i].id) + end + stack._reset() + vim.cmd("silent! %bwipeout!") + vim.fn.delete(tmpdir, "rf") + end) + + it("sends the cursor column in each client's offset encoding", function() + local sent = {} + vim.lsp.get_clients = function() + return { + make_client("utf-8", nil, sent), + make_client("utf-16", nil, sent), + make_client("utf-32", nil, sent), + } + end + + collect(lsp_provider.definition, ctx_at(0, 10)) + collect(lsp_provider.definition, ctx_at(1, 5)) + + local by_encoding = {} + for _, item in ipairs(sent) do + by_encoding[item.encoding] = by_encoding[item.encoding] or {} + table.insert(by_encoding[item.encoding], item.params.position.character) + end + assert.same({ 10, 5 }, by_encoding["utf-8"]) + assert.same({ 4, 3 }, by_encoding["utf-16"]) + assert.same({ 4, 2 }, by_encoding["utf-32"]) + end) + + it("converts response columns from each client's encoding to bytes", function() + vim.lsp.get_clients = function() + return { + make_client("utf-8", function() + return lsp_location(0, 10) + end), + make_client("utf-16", function() + return { lsp_location(0, 4), lsp_location(1, 3) } + end), + make_client("utf-32", function() + return lsp_location(1, 2) + end), + } + end + + local locations = collect(lsp_provider.references, ctx_at(2, 0)) + + assert.equals(4, #locations) + for _, loc in ipairs(locations) do + local expected = loc.range.start.line == 0 and 10 or 5 + assert.equals(expected, loc.range.start.character) + assert.equals(expected + 6, loc.range["end"].character) + end + end) + + it("reads unloaded files from disk to convert columns", function() + local other = tmpdir .. "/other.txt" + vim.fn.writefile({ "x", EMOJI_LINE }, other) + local other_uri = vim.uri_from_fname(other) + vim.lsp.get_clients = function() + return { + make_client("utf-16", function() + return lsp_location(1, 3, other_uri) + end), + } + end + + local locations = collect(lsp_provider.definition, ctx_at(2, 0)) + + assert.equals(1, #locations) + assert.equals(5, locations[1].range.start.character) + assert.equals(-1, vim.fn.bufnr(other)) + end) + + it("converts document symbol ranges to bytes", function() + vim.lsp.get_clients = function() + return { + make_client("utf-16", function() + return { + { + name = "target", + kind = 13, + range = lsp_location(0, 0).range, + selectionRange = lsp_location(0, 4).range, + }, + } + end), + } + end + + local locations = collect(lsp_provider.symbols_document, ctx_at(2, 0)) + + assert.equals(1, #locations) + assert.equals(10, locations[1].range.start.character) + assert.equals(16, locations[1].range["end"].character) + end) + + it("keeps a multibyte target that differs from the cursor only in LSP units", function() + -- The cursor sits on byte 3 ("本") and the target on UTF-16 column 3 (the + -- space at byte 9). Comparing the raw columns would drop the target. + vim.lsp.get_clients = function() + return { + make_client("utf-16", function() + return { + uri = uri, + range = { + start = { line = 0, character = 3 }, + ["end"] = { line = 0, character = 3 }, + }, + } + end), + } + end + + vim.api.nvim_win_set_cursor(0, { 1, 3 }) + peekstack.peek.definition() + + local popups = stack.list() + assert.equals(1, #popups) + assert.equals(9, popups[1].location.range.start.character) + end) + + it("opens the popup on the byte column and requests from a copy popup in client units", function() + local sent = {} + vim.lsp.get_clients = function() + return { + make_client("utf-16", function(params) + if params.position.line == 2 then + return lsp_location(0, 4) + end + return lsp_location(1, 3) + end, sent), + } + end + + vim.api.nvim_win_set_cursor(0, { 3, 0 }) + peekstack.peek.definition({ buffer_mode = "copy" }) + + local popups = stack.list() + assert.equals(1, #popups) + local popup = popups[1] + assert.equals("copy", popup.buffer_mode) + assert.same({ 1, 10 }, vim.api.nvim_win_get_cursor(popup.winid)) + + vim.api.nvim_set_current_win(popup.winid) + peekstack.peek.definition() + + assert.equals(0, sent[2].params.position.line) + assert.equals(4, sent[2].params.position.character) + popups = stack.list() + assert.equals(2, #popups) + assert.same({ 2, 5 }, vim.api.nvim_win_get_cursor(popups[2].winid)) + end) +end) From 1b1e22e1457f5cc3ceee125d6cdb50964bb89c04 Mon Sep 17 00:00:00 2001 From: Masaaki Hirotsu Date: Sat, 3 Oct 2026 08:59:30 +0900 Subject: [PATCH 2/3] fix(location): land LocationLink results on targetSelectionRange LocationLink results used targetRange, which spans the whole symbol, so popups opened at the start of the declaration instead of its name, and a request from inside a function body was dropped as the same position. The link's selection range is now the location range for both navigation and the same-position filter, and the unused originSelectionRange copy, kept in client units, is no longer stored. --- lua/peekstack/core/location.lua | 5 +++-- lua/peekstack/types.lua | 1 - tests/location_spec.lua | 18 ++++++++++++++++++ tests/same_location_filter_spec.lua | 24 ++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/lua/peekstack/core/location.lua b/lua/peekstack/core/location.lua index d83acad..74de533 100644 --- a/lua/peekstack/core/location.lua +++ b/lua/peekstack/core/location.lua @@ -158,7 +158,9 @@ local function from_lsp_location(loc, provider) return nil end local uri = loc.uri or loc.targetUri - local range = loc.range or loc.targetRange + -- A LocationLink's targetRange spans the whole symbol (e.g. a function body); + -- targetSelectionRange is the name to land on and to compare against the cursor. + local range = loc.range or loc.targetSelectionRange or loc.targetRange if not uri or not range then return nil end @@ -168,7 +170,6 @@ local function from_lsp_location(loc, provider) text = loc.text, kind = loc.kind, provider = provider, - origin = loc.originSelectionRange, } end diff --git a/lua/peekstack/types.lua b/lua/peekstack/types.lua index 1266d35..fd4c647 100644 --- a/lua/peekstack/types.lua +++ b/lua/peekstack/types.lua @@ -11,7 +11,6 @@ ---@field text? string ---@field kind? integer ---@field provider string ----@field origin? PeekstackRange ---@class PeekstackDiagnosticExtmarks ---@field bufnr integer diff --git a/tests/location_spec.lua b/tests/location_spec.lua index b129e11..6d6c367 100644 --- a/tests/location_spec.lua +++ b/tests/location_spec.lua @@ -36,6 +36,24 @@ describe("location", function() assert.equals(3, result.range.start.line) end) + it("prefers targetSelectionRange over targetRange for a LocationLink", function() + local loc = { + targetUri = "file:///tmp/bar.lua", + targetRange = { + start = { line = 3, character = 0 }, + ["end"] = { line = 9, character = 3 }, + }, + targetSelectionRange = { + start = { line = 3, character = 9 }, + ["end"] = { line = 3, character = 14 }, + }, + } + local result = location.normalize(loc, "lsp.definition") + assert.is_not_nil(result) + assert.same({ line = 3, character = 9 }, result.range.start) + assert.same({ line = 3, character = 14 }, result.range["end"]) + end) + it("normalizes a filename-based location", function() local loc = { filename = "/tmp/test.lua", diff --git a/tests/same_location_filter_spec.lua b/tests/same_location_filter_spec.lua index ec3fbd9..aab9c6a 100644 --- a/tests/same_location_filter_spec.lua +++ b/tests/same_location_filter_spec.lua @@ -74,6 +74,30 @@ describe("same location filter", function() assert.equals(loc_other.range.start.line, stack_list[1].location.range.start.line) end) + it("keeps a LocationLink whose targetRange contains the cursor", function() + local bufnr = vim.api.nvim_create_buf(false, true) + vim.api.nvim_set_current_buf(bufnr) + vim.api.nvim_buf_set_lines(bufnr, 0, -1, false, { "function foo()", " return 1", "end" }) + set_cursor(2, 2) + + peekstack.register_provider("lsp.test_filter_link", function(_ctx, cb) + cb({ + { + targetUri = vim.uri_from_bufnr(bufnr), + targetRange = { start = { line = 0, character = 0 }, ["end"] = { line = 2, character = 3 } }, + targetSelectionRange = { start = { line = 0, character = 9 }, ["end"] = { line = 0, character = 12 } }, + }, + }) + end) + + peekstack.peek("lsp.test_filter_link", {}) + + local stack_list = stack.list() + assert.equals(1, #stack_list) + assert.same({ line = 0, character = 9 }, stack_list[1].location.range.start) + vim.api.nvim_buf_delete(bufnr, { force = true }) + end) + it("does not filter for non-lsp providers", function() set_cursor(1, 0) local loc_same = helpers.make_location({ From d43226bb3cc7a5bd0ebb320a33f522f7315ed4cc Mon Sep 17 00:00:00 2001 From: Masaaki Hirotsu Date: Sat, 3 Oct 2026 09:01:00 +0900 Subject: [PATCH 3/3] fix(lsp): convert against the text Neovim shows and isolate failures Column conversion read unloaded files byte for byte and matched loaded buffers by exact URI, so a UTF-8 BOM shifted the first line, and a symlinked or differently escaped URI fell back to stale disk text instead of the unsaved buffer. - Strip the BOM and CRs when reading unloaded files, and match loaded buffers by resolved path as well as by URI. - Convert inside the per-response pcall so a malformed response no longer discards that client's slot and delays completion until the timeout. - Cover multiline ranges, LocationLink selection ranges and malformed responses alongside valid clients. --- lua/peekstack/core/location.lua | 20 ++++++- lua/peekstack/providers/lsp.lua | 9 ++- tests/lsp_offset_encoding_spec.lua | 93 ++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 5 deletions(-) diff --git a/lua/peekstack/core/location.lua b/lua/peekstack/core/location.lua index 74de533..edb51a5 100644 --- a/lua/peekstack/core/location.lua +++ b/lua/peekstack/core/location.lua @@ -225,12 +225,22 @@ function M.list_from_lsp(result, provider) return items end +---Find the loaded buffer for a URI, matching equivalent paths (symlinks, +---differently escaped URIs) so unsaved buffer text wins over the file on disk. ---@param uri string ---@return integer? local function loaded_buf_for_uri(uri) + local fname = fs.uri_to_fname(uri) + local real = fname and resolve_realpath(fname) for _, bufnr in ipairs(vim.api.nvim_list_bufs()) do - if vim.api.nvim_buf_is_loaded(bufnr) and vim.uri_from_bufnr(bufnr) == uri then - return bufnr + if vim.api.nvim_buf_is_loaded(bufnr) then + if vim.uri_from_bufnr(bufnr) == uri then + return bufnr + end + local name = vim.api.nvim_buf_get_name(bufnr) + if real and name ~= "" and resolve_realpath(name) == real then + return bufnr + end end end return nil @@ -256,9 +266,13 @@ local function line_reader(uri) local file = fname and io.open(fname, "rb") if file then for line in file:lines() do - lines[#lines + 1] = line + -- Match the buffer text Neovim would show: no CR from CRLF files. + lines[#lines + 1] = line:gsub("\r$", "") end file:close() + if lines[1] then + lines[1] = lines[1]:gsub("^\239\187\191", "") + end end end return lines[row + 1] diff --git a/lua/peekstack/providers/lsp.lua b/lua/peekstack/providers/lsp.lua index 255ad0e..519963a 100644 --- a/lua/peekstack/providers/lsp.lua +++ b/lua/peekstack/providers/lsp.lua @@ -143,9 +143,14 @@ local function request(ctx, method, provider, params_modifier, result_mapper, cb return end if not err and result then - local ok, locs = pcall(mapper, result, provider, ctx) + local ok, locs = pcall(function() + local mapped = mapper(result, provider, ctx) + if type(mapped) == "table" then + location.ranges_to_byte(mapped, client.offset_encoding) + end + return mapped + end) if ok and type(locs) == "table" then - location.ranges_to_byte(locs, client.offset_encoding) vim.list_extend(all_locations, locs) end end diff --git a/tests/lsp_offset_encoding_spec.lua b/tests/lsp_offset_encoding_spec.lua index bf6fc2b..37433cd 100644 --- a/tests/lsp_offset_encoding_spec.lua +++ b/tests/lsp_offset_encoding_spec.lua @@ -154,6 +154,99 @@ describe("lsp offset encoding", function() assert.equals(-1, vim.fn.bufnr(other)) end) + it("ignores a UTF-8 BOM and CRLF line endings in unloaded files", function() + local other = tmpdir .. "/bom.txt" + local file = assert(io.open(other, "wb")) + file:write("\239\187\191" .. CJK_LINE .. "\r\n" .. EMOJI_LINE .. "\r\n") + file:close() + local other_uri = vim.uri_from_fname(other) + vim.lsp.get_clients = function() + return { + make_client("utf-16", function() + return { + uri = other_uri, + range = { + start = { line = 0, character = 4 }, + -- Past the end of the line: clamps to the line length, not into the CR. + ["end"] = { line = 1, character = 99 }, + }, + } + end), + } + end + + local locations = collect(lsp_provider.definition, ctx_at(2, 0)) + + assert.equals(1, #locations) + assert.same({ line = 0, character = 10 }, locations[1].range.start) + assert.same({ line = 1, character = #EMOJI_LINE }, locations[1].range["end"]) + end) + + it("uses unsaved text of a loaded buffer reached through a symlinked path", function() + local link = tmpdir .. "/link.txt" + assert(vim.uv.fs_symlink(fname, link)) + -- Disk still has CJK_LINE on line 0; the buffer now has the emoji there. + vim.api.nvim_buf_set_lines(0, 0, 1, false, { EMOJI_LINE }) + vim.lsp.get_clients = function() + return { + make_client("utf-16", function() + return lsp_location(0, 3, vim.uri_from_fname(link)) + end), + } + end + + local locations = collect(lsp_provider.definition, ctx_at(2, 0)) + + assert.equals(1, #locations) + assert.equals(5, locations[1].range.start.character) + end) + + it("keeps other clients' results when one response cannot be converted", function() + vim.lsp.get_clients = function() + return { + make_client("utf-16", function() + return { uri = uri, range = { start = { line = 0, character = 4 } } } + end), + make_client("utf-16", function() + -- A non-string URI makes the conversion itself throw. + return { uri = 42, range = lsp_location(0, 4).range } + end), + make_client("utf-16", function() + return lsp_location(1, 3) + end), + } + end + + local locations = collect(lsp_provider.definition, ctx_at(2, 0)) + + assert.equals(2, #locations) + assert.same({ line = 0, character = 10 }, locations[1].range.start) + assert.same({ line = 0, character = 10 }, locations[1].range["end"]) + assert.same({ line = 1, character = 5 }, locations[2].range.start) + end) + + it("converts a LocationLink's targetSelectionRange", function() + vim.lsp.get_clients = function() + return { + make_client("utf-16", function() + return { + { + targetUri = uri, + targetRange = { start = { line = 0, character = 0 }, ["end"] = { line = 1, character = 9 } }, + targetSelectionRange = { start = { line = 1, character = 3 }, ["end"] = { line = 1, character = 9 } }, + }, + } + end), + } + end + + local locations = collect(lsp_provider.definition, ctx_at(2, 0)) + + assert.equals(1, #locations) + assert.same({ line = 1, character = 5 }, locations[1].range.start) + assert.same({ line = 1, character = 11 }, locations[1].range["end"]) + end) + it("converts document symbol ranges to bytes", function() vim.lsp.get_clients = function() return {