📝 docs(plan): implement UTF-16 text coordinate conversion
This commit is contained in:
@@ -0,0 +1,409 @@
|
||||
# LSP UTF-16 Text Coordinates Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Convert LSP UTF-16 positions into consistent UTF-8 byte offsets and Tree-sitter byte points.
|
||||
|
||||
**Architecture:** `lsp.utils.text_coordinates` exposes one `ToBytePosition` operation that resolves both representations in a single scan, while `ToOffset` remains a wrapper for provider callers. Incremental parsing consumes the paired result so `TSInputEdit` byte offsets and points always refer to the same source location.
|
||||
|
||||
**Tech Stack:** C++23 Modules, LSP 3.17 UTF-16 positions, UTF-8, Tree-sitter, CMake, CTest
|
||||
|
||||
---
|
||||
|
||||
## Plan Meta
|
||||
|
||||
- **Plan Group:** text-coordinates-utf16
|
||||
- **Parent Plan:** none
|
||||
- **Verification Scope:** text-coordinate regression tests, provider test suite, server build, LSP transport smoke test
|
||||
- **Verification Gate:** `test_provider` passes, `tsl-server` builds, changed C++ files pass clang-format, and the LSP JSON transport completes with response validation disabled
|
||||
- **Execution Constraints:** `karpathy-guidelines`, `.agents`, `AGENT_RULES.md`, test-driven development
|
||||
|
||||
## File Map
|
||||
|
||||
- Create `lsp-server/test/test_provider/text_coordinates_test.cppm`: focused UTF-16-to-byte coordinate regression tests.
|
||||
- Modify `lsp-server/test/test_provider/CMakeLists.txt`: compile the new test module in `test_provider`.
|
||||
- Modify `lsp-server/test/test_provider/test_main.cppm`: register the text-coordinate tests.
|
||||
- Modify `lsp-server/src/utils/text_coordinates.cppm`: add paired byte-position resolution and correct UTF-16 decoding.
|
||||
- Modify `lsp-server/src/manager/parser.cppm`: build `TSInputEdit` from paired coordinate results.
|
||||
|
||||
### Task 1: Drive UTF-16 coordinate conversion with regression tests
|
||||
|
||||
**Files:**
|
||||
|
||||
- Create: `lsp-server/test/test_provider/text_coordinates_test.cppm`
|
||||
- Modify: `lsp-server/test/test_provider/CMakeLists.txt`
|
||||
- Modify: `lsp-server/test/test_provider/test_main.cppm`
|
||||
|
||||
- [ ] **Step 1: Add the focused test module**
|
||||
|
||||
Create `lsp-server/test/test_provider/text_coordinates_test.cppm`:
|
||||
|
||||
```cpp
|
||||
module;
|
||||
|
||||
export module lsp.test.provider.text_coordinates;
|
||||
|
||||
import std;
|
||||
|
||||
import lsp.protocol;
|
||||
import lsp.test.framework;
|
||||
import lsp.utils.text_coordinates;
|
||||
|
||||
export namespace lsp::test::provider
|
||||
{
|
||||
class TextCoordinatesTests
|
||||
{
|
||||
public:
|
||||
static void Register(TestRunner& runner);
|
||||
|
||||
private:
|
||||
static TestResult TestAsciiAndBmpPositions();
|
||||
static TestResult TestSupplementaryPlanePositions();
|
||||
static TestResult TestMultilineAndClampedPositions();
|
||||
static TestResult TestMalformedUtf8IsBounded();
|
||||
static TestResult TestCalculateEndPointUsesByteColumns();
|
||||
};
|
||||
}
|
||||
|
||||
namespace lsp::test::provider
|
||||
{
|
||||
namespace
|
||||
{
|
||||
void ExpectPosition(const utils::text_coordinates::BytePosition& actual,
|
||||
protocol::uinteger offset,
|
||||
std::uint32_t row,
|
||||
std::uint32_t column)
|
||||
{
|
||||
assertEqual(offset, actual.offset, "byte offset should match");
|
||||
assertEqual(row, actual.point.row, "Tree-sitter row should match");
|
||||
assertEqual(column, actual.point.column, "Tree-sitter byte column should match");
|
||||
}
|
||||
}
|
||||
|
||||
void TextCoordinatesTests::Register(TestRunner& runner)
|
||||
{
|
||||
runner.addTest("text coordinates convert ASCII and BMP positions", TestAsciiAndBmpPositions);
|
||||
runner.addTest("text coordinates convert supplementary-plane positions", TestSupplementaryPlanePositions);
|
||||
runner.addTest("text coordinates clamp multiline positions", TestMultilineAndClampedPositions);
|
||||
runner.addTest("text coordinates bound malformed UTF-8", TestMalformedUtf8IsBounded);
|
||||
runner.addTest("text coordinates calculate byte end points", TestCalculateEndPointUsesByteColumns);
|
||||
}
|
||||
|
||||
TestResult TextCoordinatesTests::TestAsciiAndBmpPositions()
|
||||
{
|
||||
const protocol::string content = "A中Z";
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 0 }, content), 0U, 0U, 0U);
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 1 }, content), 1U, 0U, 1U);
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 2 }, content), 4U, 0U, 4U);
|
||||
assertEqual(4U, utils::text_coordinates::ToOffset({ 0, 2 }, content),
|
||||
"ToOffset should use the paired conversion");
|
||||
return { "", true, "ok" };
|
||||
}
|
||||
|
||||
TestResult TextCoordinatesTests::TestSupplementaryPlanePositions()
|
||||
{
|
||||
const protocol::string content = "A😀Z";
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 1 }, content), 1U, 0U, 1U);
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 2 }, content), 1U, 0U, 1U);
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 3 }, content), 5U, 0U, 5U);
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 4 }, content), 6U, 0U, 6U);
|
||||
return { "", true, "ok" };
|
||||
}
|
||||
|
||||
TestResult TextCoordinatesTests::TestMultilineAndClampedPositions()
|
||||
{
|
||||
const protocol::string content = "中x\n😀y";
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 1, 0 }, content), 5U, 1U, 0U);
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 1, 2 }, content), 9U, 1U, 4U);
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 1, 100 }, content), 10U, 1U, 5U);
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 9, 0 }, content), 10U, 1U, 5U);
|
||||
return { "", true, "ok" };
|
||||
}
|
||||
|
||||
TestResult TextCoordinatesTests::TestMalformedUtf8IsBounded()
|
||||
{
|
||||
const protocol::string content{ 'A', static_cast<char>(0xF0), static_cast<char>(0x9F), 'Z' };
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 2 }, content), 2U, 0U, 2U);
|
||||
ExpectPosition(utils::text_coordinates::ToBytePosition({ 0, 100 }, content), 4U, 0U, 4U);
|
||||
return { "", true, "ok" };
|
||||
}
|
||||
|
||||
TestResult TextCoordinatesTests::TestCalculateEndPointUsesByteColumns()
|
||||
{
|
||||
const auto end = utils::text_coordinates::CalculateEndPoint("中\n😀x", { 2U, 3U });
|
||||
assertEqual(3U, end.row, "newline should advance the Tree-sitter row");
|
||||
assertEqual(5U, end.column, "multibyte text should advance the byte column");
|
||||
return { "", true, "ok" };
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Register the new test module in the provider target**
|
||||
|
||||
Add `text_coordinates_test.cppm` beside the other test modules in both `SOURCES` and the CMake `FILE_SET` in `lsp-server/test/test_provider/CMakeLists.txt`:
|
||||
|
||||
```cmake
|
||||
text_coordinates_test.cppm
|
||||
```
|
||||
|
||||
Import and register it in `lsp-server/test/test_provider/test_main.cppm`:
|
||||
|
||||
```cpp
|
||||
import lsp.test.provider.text_coordinates;
|
||||
```
|
||||
|
||||
```cpp
|
||||
std::cout << " - Text coordinate tests" << std::endl;
|
||||
lsp::test::provider::TextCoordinatesTests::Register(runner);
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Build to verify the test is red**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cmake --build lsp-server/build/codex43-clean/Release --target test_provider -j2
|
||||
```
|
||||
|
||||
Expected: build fails because `BytePosition` and `ToBytePosition` do not exist. This proves the test requires the new paired conversion API.
|
||||
|
||||
### Task 2: Implement one UTF-16-to-byte conversion path
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `lsp-server/src/utils/text_coordinates.cppm`
|
||||
- Modify: `lsp-server/src/manager/parser.cppm`
|
||||
|
||||
- [ ] **Step 1: Replace the independent point conversion with paired resolution**
|
||||
|
||||
In the exported namespace of `lsp-server/src/utils/text_coordinates.cppm`, replace `ToPoint` with:
|
||||
|
||||
```cpp
|
||||
struct BytePosition
|
||||
{
|
||||
protocol::uinteger offset;
|
||||
TSPoint point;
|
||||
};
|
||||
|
||||
BytePosition ToBytePosition(const protocol::Position& position, const protocol::string& content);
|
||||
```
|
||||
|
||||
In the implementation namespace, add a validated UTF-8 decoder and implement the paired conversion:
|
||||
|
||||
```cpp
|
||||
namespace
|
||||
{
|
||||
struct DecodedCharacter
|
||||
{
|
||||
std::size_t byte_count;
|
||||
protocol::uinteger utf16_units;
|
||||
};
|
||||
|
||||
DecodedCharacter DecodeCharacter(std::string_view content, std::size_t offset)
|
||||
{
|
||||
const auto lead = static_cast<unsigned char>(content[offset]);
|
||||
if ((lead & 0x80U) == 0)
|
||||
return { 1, 1 };
|
||||
|
||||
std::size_t byte_count = 0;
|
||||
std::uint32_t code_point = 0;
|
||||
std::uint32_t minimum = 0;
|
||||
if ((lead & 0xE0U) == 0xC0U)
|
||||
{
|
||||
byte_count = 2;
|
||||
code_point = lead & 0x1FU;
|
||||
minimum = 0x80U;
|
||||
}
|
||||
else if ((lead & 0xF0U) == 0xE0U)
|
||||
{
|
||||
byte_count = 3;
|
||||
code_point = lead & 0x0FU;
|
||||
minimum = 0x800U;
|
||||
}
|
||||
else if ((lead & 0xF8U) == 0xF0U)
|
||||
{
|
||||
byte_count = 4;
|
||||
code_point = lead & 0x07U;
|
||||
minimum = 0x10000U;
|
||||
}
|
||||
else
|
||||
{
|
||||
return { 1, 1 };
|
||||
}
|
||||
|
||||
if (byte_count > content.size() - offset)
|
||||
return { 1, 1 };
|
||||
|
||||
for (std::size_t index = 1; index < byte_count; ++index)
|
||||
{
|
||||
const auto continuation = static_cast<unsigned char>(content[offset + index]);
|
||||
if ((continuation & 0xC0U) != 0x80U)
|
||||
return { 1, 1 };
|
||||
code_point = (code_point << 6U) | (continuation & 0x3FU);
|
||||
}
|
||||
|
||||
if (code_point < minimum || code_point > 0x10FFFFU ||
|
||||
(code_point >= 0xD800U && code_point <= 0xDFFFU))
|
||||
{
|
||||
return { 1, 1 };
|
||||
}
|
||||
|
||||
return { byte_count, code_point >= 0x10000U ? 2U : 1U };
|
||||
}
|
||||
}
|
||||
|
||||
BytePosition ToBytePosition(const protocol::Position& position, const protocol::string& content)
|
||||
{
|
||||
std::size_t offset = 0;
|
||||
std::size_t line_start = 0;
|
||||
std::uint32_t row = 0;
|
||||
|
||||
while (offset < content.size() && row < position.line)
|
||||
{
|
||||
if (content[offset++] == '\n')
|
||||
{
|
||||
++row;
|
||||
line_start = offset;
|
||||
}
|
||||
}
|
||||
|
||||
protocol::uinteger utf16_units = 0;
|
||||
while (offset < content.size() && content[offset] != '\n' &&
|
||||
utf16_units < position.character)
|
||||
{
|
||||
const auto decoded = DecodeCharacter(content, offset);
|
||||
const auto remaining = position.character - utf16_units;
|
||||
if (decoded.utf16_units > remaining)
|
||||
break;
|
||||
|
||||
offset += decoded.byte_count;
|
||||
utf16_units += decoded.utf16_units;
|
||||
}
|
||||
|
||||
return {
|
||||
.offset = static_cast<protocol::uinteger>(offset),
|
||||
.point = TSPoint{
|
||||
.row = row,
|
||||
.column = static_cast<std::uint32_t>(offset - line_start),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
protocol::uinteger ToOffset(const protocol::Position& position, const protocol::string& content)
|
||||
{
|
||||
return ToBytePosition(position, content).offset;
|
||||
}
|
||||
```
|
||||
|
||||
Keep `CalculateEndPoint` unchanged: Tree-sitter columns count UTF-8 bytes, so its byte iteration is correct.
|
||||
|
||||
- [ ] **Step 2: Use paired positions for incremental Tree-sitter edits**
|
||||
|
||||
Replace the separate offset and point conversions in `SyntaxTree::ApplyEdit` in `lsp-server/src/manager/parser.cppm`:
|
||||
|
||||
```cpp
|
||||
const auto start = utils::text_coordinates::ToBytePosition(change.range.start, content);
|
||||
const auto old_end = utils::text_coordinates::ToBytePosition(change.range.end, content);
|
||||
|
||||
TSInputEdit edit{};
|
||||
edit.start_byte = start.offset;
|
||||
edit.old_end_byte = old_end.offset;
|
||||
edit.new_end_byte = start.offset + change.text.length();
|
||||
edit.start_point = start.point;
|
||||
edit.old_end_point = old_end.point;
|
||||
edit.new_end_point = utils::text_coordinates::CalculateEndPoint(change.text, edit.start_point);
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Build and run the regression suite to verify green**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cmake --build lsp-server/build/codex43-clean/Release --target test_provider -j2
|
||||
ctest --test-dir lsp-server/build/codex43-clean/Release -R '^test_provider$' --output-on-failure
|
||||
```
|
||||
|
||||
Expected: the build succeeds and CTest reports `100% tests passed, 0 tests failed out of 1`.
|
||||
|
||||
- [ ] **Step 4: Format the changed C++ files and verify formatting**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
clang-format -i \
|
||||
lsp-server/src/utils/text_coordinates.cppm \
|
||||
lsp-server/src/manager/parser.cppm \
|
||||
lsp-server/test/test_provider/text_coordinates_test.cppm \
|
||||
lsp-server/test/test_provider/test_main.cppm
|
||||
clang-format --dry-run --Werror \
|
||||
lsp-server/src/utils/text_coordinates.cppm \
|
||||
lsp-server/src/manager/parser.cppm \
|
||||
lsp-server/test/test_provider/text_coordinates_test.cppm \
|
||||
lsp-server/test/test_provider/test_main.cppm
|
||||
```
|
||||
|
||||
Expected: both commands exit successfully with no diagnostics.
|
||||
|
||||
- [ ] **Step 5: Commit the implementation**
|
||||
|
||||
```bash
|
||||
git add \
|
||||
lsp-server/src/utils/text_coordinates.cppm \
|
||||
lsp-server/src/manager/parser.cppm \
|
||||
lsp-server/test/test_provider/CMakeLists.txt \
|
||||
lsp-server/test/test_provider/test_main.cppm \
|
||||
lsp-server/test/test_provider/text_coordinates_test.cppm
|
||||
git commit -m "fix: convert LSP UTF-16 text coordinates"
|
||||
```
|
||||
|
||||
### Task 3: Verify server integration
|
||||
|
||||
**Files:**
|
||||
|
||||
- Verify only; no additional source changes expected.
|
||||
|
||||
- [ ] **Step 1: Build the production server and provider tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cmake --build lsp-server/build/codex43-clean/Release --target tsl-server test_provider -j2
|
||||
```
|
||||
|
||||
Expected: Ninja completes successfully.
|
||||
|
||||
- [ ] **Step 2: Run provider and LSP transport verification**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
ctest --test-dir lsp-server/build/codex43-clean/Release -R '^test_provider$' --output-on-failure
|
||||
python lsp-server/test/run_lsp_json_tests.py \
|
||||
--server lsp-server/build/codex43-clean/Release/tsl-server \
|
||||
--no-validate
|
||||
```
|
||||
|
||||
Expected: `test_provider` reports zero failures and the LSP script completes all request/response exchanges successfully. Response validation remains disabled because the repository has a known unrelated rename-response fixture failure.
|
||||
|
||||
- [ ] **Step 3: Confirm the old point-only API has no callers**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
rg -n 'text_coordinates::ToPoint|text::ToPoint|TSPoint ToPoint' lsp-server/src lsp-server/test
|
||||
```
|
||||
|
||||
Expected: no matches.
|
||||
|
||||
- [ ] **Step 4: Record completion through the repository main loop**
|
||||
|
||||
After all verification gates pass, run:
|
||||
|
||||
```bash
|
||||
python docs/standards/playbook/scripts/main_loop.py finish \
|
||||
-plan docs/superpowers/plans/2026-07-12-text-coordinates-utf16.md \
|
||||
-status done \
|
||||
-progress memory-bank/progress.md
|
||||
```
|
||||
|
||||
Expected: the plan is recorded as `done`. Update the human-readable summary in `memory-bank/progress.md` to mention the UTF-16 coordinate fix and its passing verification, then commit that state update separately.
|
||||
@@ -50,7 +50,7 @@
|
||||
<!-- workflow-state:start -->
|
||||
phase: planning
|
||||
spec: docs/superpowers/specs/2026-07-12-text-coordinates-utf16-design.md
|
||||
plan: docs/superpowers/plans/2026-07-11-args-parser-startup-errors.md
|
||||
plan: docs/superpowers/plans/2026-07-12-text-coordinates-utf16.md
|
||||
executor: executing-plans
|
||||
constraints: karpathy-guidelines,.agents,AGENT_RULES
|
||||
<!-- workflow-state:end -->
|
||||
|
||||
Reference in New Issue
Block a user