Follow-up from the comprehensive review of PR #239 (LSP server). Line numbers reference the PR branch at review time.
Severity: major (1), minor (2-3), nit (4-5) — documentation that contradicts the code it documents.
ide/base-db/README.md:58 describes LineIndex line-splitting semantics that are the OPPOSITE of the implementation: it claims a lone \r is not a line terminator and stays in the preceding line's content, but the code (correctly) implements the LSP 3.17 EOL set — for "a\rb" the README predicts one line, the code returns two (line_col(2) == LineCol{line:1, character:0}). The Testing section (line 103) compounds it by claiming a test covers "\r staying inside its line's content" when the test asserts the opposite (line_index.rs:254-260). Anyone building on LineIndex from the README computes wrong expectations for classic-Mac/mixed-EOL files.apps/lsp/README.md:32 layering claim is false: "Only ide-db depends on the compiler crates; apps/lsp and ide/ide never name a compiler type" — but inference-ide's Cargo.toml depends on inference-ast and inference-type-checker, and goto_definition/completions/diagnostics/document_symbols/inlay_hints/syntax/type_render all import compiler types extensively. A contributor planning a compiler-internal change would mis-scope the blast radius.- CHANGELOG.md:798 claims the LSP e2e suite covers "18 scenarios"; the suite in the same PR has 27 test functions, which
apps/lsp/README.md:226 itself groups as twenty-one scenarios. Stale count.
- CHANGELOG.md:813
"never `[FAIL]`" is overstated for the doctor inference-lsp check: absence of the server is never a failure, but the check does return error status (rendered [FAIL]) on platform-detection/toolchain-path/default-version read failures.
ide/vfs/README.md:89 API table documents set_contents(FileId, impl Into<Arc<str>>) but the signature takes Arc<str> directly (the Into sugar lives one layer up).
Follow-up from the comprehensive review of PR #239 (LSP server). Line numbers reference the PR branch at review time.
Severity: major (1), minor (2-3), nit (4-5) — documentation that contradicts the code it documents.
ide/base-db/README.md:58describes LineIndex line-splitting semantics that are the OPPOSITE of the implementation: it claims a lone\ris not a line terminator and stays in the preceding line's content, but the code (correctly) implements the LSP 3.17 EOL set — for"a\rb"the README predicts one line, the code returns two (line_col(2) == LineCol{line:1, character:0}). The Testing section (line 103) compounds it by claiming a test covers "\rstaying inside its line's content" when the test asserts the opposite (line_index.rs:254-260). Anyone building on LineIndex from the README computes wrong expectations for classic-Mac/mixed-EOL files.apps/lsp/README.md:32layering claim is false: "Onlyide-dbdepends on the compiler crates;apps/lspandide/idenever name a compiler type" — butinference-ide's Cargo.toml depends oninference-astandinference-type-checker, and goto_definition/completions/diagnostics/document_symbols/inlay_hints/syntax/type_render all import compiler types extensively. A contributor planning a compiler-internal change would mis-scope the blast radius.apps/lsp/README.md:226itself groups as twenty-one scenarios. Stale count."never `[FAIL]`"is overstated for the doctor inference-lsp check: absence of the server is never a failure, but the check does return error status (rendered[FAIL]) on platform-detection/toolchain-path/default-version read failures.ide/vfs/README.md:89API table documentsset_contents(FileId, impl Into<Arc<str>>)but the signature takesArc<str>directly (the Into sugar lives one layer up).