bump LSP features [AI] - #76
Merged
Merged
Conversation
With full djot conformance in oymarkit, explicit {#id} attributes on
inline spans, blocks, and headings become a first-class anchor kind
alongside heading slugs and caret block ids. See the new spec
docs/feature-attribute-anchors.mld.
Core (lib/vault):
- Index.extract_attr_ids collects ids from Block.Ext_attributes,
Inline.Ext_attributes, and explicit heading `Id, with the wrapped
content's textloc (column-precise for inline spans).
- Resolve gains Attr / Curr_attr targets; #frag falls back to attribute
ids after slug and caret lookups, one shared namespace, first match
wins.
- Threaded through embed (non-embeddable for now), component/html url
rendering, and vault_graph (new Attr vertex).
LSP (lsp_lib):
- go-to-definition resolves #id to attribute anchors (line-granular).
- find-references: new Path_attr target; backlinks from a link, plus
standalone {#id} block-attribute line detection.
- hover falls back to the target file for attribute anchors.
Also fixes parse.ml for the oymarkit API rename
(djot_inline_attributes -> inline_attributes, likewise block).
Tests: extraction, cross/self-file gtd, and find-references.
go-to-definition now returns a (line, character) position instead of just
a line. The character is derived from the resolved node's Cmarkit.Textloc
(first_byte - line start byte) with no target-file read, since line_pos
already carries the line's start byte. Inline attribute anchors ({#id} on
a span) therefore land mid-line; headings, blocks, and block-level
anchors stay at column 0 as before.
- Lsp_util.position_of_textloc: Textloc -> (line, character), byte column
(ASCII-correct pending feature-utf16-positions).
- definition_result gains `character`; main.ml plumbs it to the wire.
- Refines feature-go-to-definition.mld: attribute-anchor rows + a
target_position section documenting the derivation and utf16 caveat.
Tests: unit column derivation, unit gtd (cross/self-file), and an E2E
JSON-RPC case (anchor-source/anchor-target fixtures) asserting character 4.
Anchor ids share one per-file namespace across heading slugs, caret block
ids, and djot attribute ids. Since djot attribute merge is override, a
repeated id makes every #id reference ambiguous. Diagnostics now flags
every located occurrence of any id that appears more than once — covering
same-kind duplicates (two {#foo}) and cross-kind collisions (a heading
whose slug equals a hand-written {#foo}).
- collect_anchor_occurrences gathers (id, range) from extract_headings /
extract_block_ids / extract_attr_ids on the freshly-parsed doc.
- compute merges these with unresolved-link diagnostics and returns the
list sorted by range start.
- Refines feature-diagnostics.mld with a duplicate_ids section.
Tests: unique/distinct (no diag), duplicate attr ids, heading-slug vs
attr-id collision.
LSP positions use UTF-16 code-unit offsets; the server previously treated character as a byte offset, wrong for any non-ASCII line. lsp_util's byte_offset_of_position and position_of_byte_offset now take ?encoding (Utf8 | Utf16 | Utf32, default Utf16) and decode UTF-8 via Stdlib.String.get_utf_8_uchar, counting code units per encoding. Defaulting to Utf16 makes every existing caller (gtd/hover/find-references cursor input, diagnostics/references result ranges) correct with no signature change. main.ml advertises positionEncoding = utf-16. Known gap (documented in feature-utf16-positions.mld): the gtd target column via position_of_textloc stays a byte column, since it has no target content to decode; correcting it needs gtd to read the target file. Tests: multibyte and surrogate-pair conversions, utf-8/utf-32 variants, round-trip, and a discriminating UTF-16 cursor case in go_to_definition.
…ver)
Adds Parse.Extract.get_block_by_attr_id: given an id, returns the block
carrying {#id} — the wrapped block for a block attribute, or the
containing paragraph/heading for an inline attribute; containers (quotes,
lists) are searched recursively.
Wires it into the two consumers that previously stubbed attribute anchors:
- embed.ml: ![[note#id]] / ![[#id]] now expand the attributed block
instead of being non-embeddable.
- hover.ml: hovering a link to an attribute anchor previews its
containing block (parsed from live content and re-rendered) instead of
dumping the whole file.
Tests: extractor unit tests (inline/block/not-found), hover attr-anchor
fragment, and E2E embed of block and inline attribute anchors.
go-to-definition now returns a UTF-16-encoded target column, completing the position-encoding story. position_of_textloc gains an optional ?content: given the target's frontmatter-stripped body it decodes the line's bytes to a UTF-16 column; without it, degrades to a byte column. go_to_definition gains ?read_file (default: always None). Cross-file targets (Heading/Block/Attr) read the target file via it; self-file targets (Curr_*) use the current buffer. Content is stripped through Parse.Frontmatter.of_string so byte offsets line up with the parse. main.ml passes self#read_file. Known limitation (documented in the gtd + tracking docs): because Textloc offsets are body-relative, the returned line is also body-relative and is short by the frontmatter line count for files with YAML frontmatter — pre-existing, needs full-file line positions from the parser. Tests: content-based UTF-16 column (unit), cross-file CJK target column, and a frontmatter guard confirming body-stripped decoding.
Textloc offsets were relative to the frontmatter-stripped body, so every LSP position in a file with YAML frontmatter was short by the frontmatter size — go-to-definition jumped to the wrong line, and find-references / diagnostics / hover ranges were misplaced. Root fix in the parser: Frontmatter.blank_frontmatter replaces the frontmatter region with whitespace (newlines kept, other bytes -> spaces) instead of stripping it, preserving every byte and line position. of_string parses this blanked input, so parsed Textlocs are full-file-relative. The leading blank lines that stand in for the frontmatter rows are dropped during assembly so rendering (HTML, CommonMark round-trip) is unchanged. This fixes all Textloc consumers at once. go_to_definition no longer strips frontmatter (uses raw content directly); the gtd frontmatter target now resolves to its true full-file line. Tests: blank_frontmatter position-preservation (unit), gtd frontmatter target at full-file line 5, and a diagnostics link range proven full-file-relative under frontmatter.
…te ids)
Implements the completion feature (previously unbuilt). Inside [[ ... ]]:
- note-name mode (no #): every vault note, labelled by its shortest
unambiguous name (basename, or full path when the basename repeats),
detail = full path, sorted alphabetically.
- fragment mode (after #): headings (by slug), caret block ids, and
explicit djot attribute ids ({#id}) of the target note — one shared
namespace. The note part is resolved via the vault resolver; the empty
note part means the current file (parsed live).
Trigger detection scans back to the innermost open [[ (also matches ![[),
bailing if a ] closes it first.
Wiring: main.ml overrides on_req_completion (the dedicated linol hook, not
on_request_unhandled) and advertises completionProvider with trigger
characters [ and #. Registered in lsp_lib; refined feature-completion.mld
with the attribute-id delta and anchor labels.
Tests: 7 unit cases (note-name disambiguation, fragment with all three
kinds, current-file, embed trigger, unresolved note, not-in-wikilink,
closed wikilink) and an E2E JSON-RPC case asserting a heading + attribute
id come back over the wire.
The LSP tests spawned the real oystermark-lsp binary and spoke JSON-RPC over a pipe, because every handler lived inside the linol server object in main.ml and stdio was the only way to reach it. That put message framing, notification filtering and a select() read timeout between each test and its assertion, and turned a server crash into a hung runner rather than an exception. Move the handler bodies into a new Lsp_lib.Server: vault state, the open buffer map, byte-offset -> Position conversion, URI construction, and WorkspaceEdit/Diagnostic shaping. It depends on linol.lsp for protocol types but not on the transport, so tests drive it directly. main.ml keeps only capability advertisement and param unwrapping. Two mechanical consequences of the move: - open_docs goes from a path set to a path -> content table. The buffer content previously came from linol's doc_state for hover, definition and completion; same source, now owned by Server, which is what makes it visible to tests. - did_save returns (rel_path, diagnostics) sorted by path instead of iterating a hash set, so main.ml addresses each publishDiagnostics. Behavior is otherwise unchanged: every pre-existing expect output matched without promotion. feature-document-sync.mld claimed open_docs did not track buffer contents and that all feature handlers read from disk. The latter was already wrong before this change; both now describe the buffer/disk split as implemented.
Two bugs in Lsp_lib.Rename, both of which corrupted files rather than
merely returning a wrong answer, and both of which the expect tests had
blessed because they only asserted on each edit's new_text and never on
the document the edits produce.
destination_bounds returns offsets relative to the link's slice. The
Path_only branch shadowed `finish` with a destination-relative value, so
`start + finish` was right there; the fragment branch reused the outer,
already slice-relative `finish` and still added `start`. Every heading,
block and attribute rename therefore overran its end by exactly the
delimiter width (2 for [[, 3 for , eating the closing
delimiter and whatever followed:
[[target#Old Heading|alias]] -> [[target#New Headinglias]]
which is the opposite of what feature-rename.mld promises about
preserving aliases and fragments. Rename the two frames apart --
dest_start/dest_stop for slice-relative, target_stop for the
destination-relative shadow -- so they can no longer be conflated, and
document destination_bounds' contract.
Separately, find_definition_line located an attribute anchor's
definition with a bare `String.is_substring text ~substring:("#" ^ id)`.
That matched a longer id sharing the prefix ({#note} found {#note-
extended} and rewrote its prefix), and matched a link fragment
([[note#id]]) which is a reference, not a definition -- after which the
reference scan edited the same span again, producing overlapping edits
in one WorkspaceEdit. Add attr_id_offset, which finds {...} spans and
parses each with Cb_attribute.of_string instead of string-searching, with
an id-char boundary check inside the body. definition_edit uses it too,
so the two can no longer disagree about which line is the definition.
Add Lsp_helper.apply_edits, which applies edits right-to-left and raises
on overlap, and assert the resulting document text in every rename test.
That is the assertion that catches a correct new_text at a wrong range.
Also document the mixed position frame the disk-reading handlers sit on
(feature-document-sync.mld, cross-referenced from server.mli). The
existing open question framed the buffer/disk split as staleness; for
rename and code actions it is worse than that. The incoming position is
always a buffer coordinate, so indexing it into disk content is not one
save behind, it is meaningless -- and those two handlers return edits the
client applies back to the buffer. Not fixed here; the narrow fix is
recorded next to the hazard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.