Package documentation for parser and server#129
Conversation
spoenemann
commented
Jul 21, 2026
spoenemann
commented
- Added parser/doc.go
- Added server/doc.go and added missing documentation of public elements
- Extended util/parallel/doc.go
- Added package doc references to AGENTS.md
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: cfdc227 | Previous: d803417 | Ratio |
|---|---|---|---|
BenchmarkWorkspaceCycle (typefox.dev/fastbelt/examples/statemachine) - MB/s |
12.53 MB/s |
5.21 MB/s |
2.40 |
This comment was automatically generated by workflow using github-action-benchmark.
sailingKieler
left a comment
sailingKieler
left a comment
There was a problem hiding this comment.
Thanks a lot @spoenemann for these documentations.
I have a few (nitpick) remarks and found one gap (actually two) while reading the docs.
| // [typefox.dev/fastbelt/workspace] — parsed, linked, and validated documents — | ||
| // to editors and IDEs: text documents are synchronized through LSP | ||
| // notifications, and language features such as completion, go to definition, | ||
| // find references, hover, and rename are answered from the workspace. |
There was a problem hiding this comment.
This sentence sounds a bit odd, e.g.
... are answered from the workspace.
| // mylang.SetupServices(sc) // language, textdoc, linking, workspace services | ||
| // mylang.SetupGeneratedServerServices(sc) | ||
| // server.SetupDefaultServices(sc) | ||
| // server.SetupStdioServices(sc) |
There was a problem hiding this comment.
Currently, we have it everywhere like this. However, intuitively I would expect the following pattern starting with basic/default definitions followed by the specific ones, esp. since there is a function service.Override[...](...) now.
server.SetupDefaultServices(sc)
server.SetupStdioServices(sc)
mylang.SetupGeneratedServerServices(sc)
mylang.SetupServices(sc) // language, textdoc, linking, workspace services
What do you think?
There was a problem hiding this comment.
At that I noticed that mylang.SetupGeneratedServerServices(sc) introduced within #79 is missing in examples/arithmetics/server/main.go, since it is also missing in the scaffold template in internal/scaffold/templates/cmd/main.go.tmpl.
Would you complete those gaps?
| sc *service.Container | ||
| } | ||
|
|
||
| // NewDefaultDocumentHighlightProvider creates a new default document highlight provider. |
There was a problem hiding this comment.
Nitpick, here and below:
| // NewDefaultDocumentHighlightProvider creates a new default document highlight provider. | |
| // NewDefaultDocumentHighlightProvider creates a new [DefaultDocumentHighlightProvider]. |
| // DefaultLanguageServer is the default implementation of the [lsp.Server] | ||
| // interface. It contains no feature logic itself: each supported LSP method | ||
| // is delegated to a dedicated service (such as [CompletionProvider] or | ||
| // [DocumentSyncher]) looked up in the service container, and request-handling |
There was a problem hiding this comment.
The DocumentSyncher is a bit of an unfavorable example wrt. to the beginning of the sentence, DefinitionProvider or DocumentSymbolProvider are a bit better suited I think.
| // connection is closed, which the default server triggers on the LSP exit | ||
| // notification. | ||
| // | ||
| // A typical main function for a generated language looks like this: |
There was a problem hiding this comment.
| // A typical main function for a generated language looks like this: | |
| // A typical main function of a language server looks like this: |
msujew
left a comment
msujew
left a comment
There was a problem hiding this comment.
I feel like this is rather wordy. I've picked some examples which I didn't like, but I feel like we would do well to cut down a bit. It explains too much of the inner workings, less of the API (which is the more interesting part to readers).
| // a custom implementation was registered first. During document processing, | ||
| // [typefox.dev/fastbelt/workspace.DefaultDocumentParser] obtains the [Parser], | ||
| // calls Parse on the tokens produced by [typefox.dev/fastbelt/lexer], and | ||
| // stores the resulting root [core.AstNode] and syntax errors ([ParseResult]) | ||
| // on the document. |
There was a problem hiding this comment.
Suggestion: Remove the whole last sentence here. It's not part of the generated parser infrastructure and I would like to change this soon, because I'm not happy with how this currently works. Having this documentation here would lead to me having more work in the future.
| // the follow set after unwinding from a failed rule (Recover). Because | ||
| // generated AST nodes tolerate nil tokens, missing tokens are reported | ||
| // but never fabricated. |
There was a problem hiding this comment.
I don't understand the last sentence. Maybe just say: "The error recovery never generates synthetic tokens for the purpose of a valid AST"?
| // 1. The generated CompletionParser reparses the document's tokens up to the | ||
| // cursor. It mirrors the main parser's control flow but mutates no AST; | ||
| // via [CompletionParserState] it records an [ATNSnapshot] — token index, | ||
| // ATN state, rule stack — at every rule entry and sync point. | ||
| // 2. [CompletionParseResult.SimulateAt] picks a suitable snapshot and | ||
| // advances an NFA-style live set over the ATN ([RuntimeATN.Simulate]) | ||
| // through the remaining tokens. Unlike prediction, the simulation keeps | ||
| // every alternative alive: the user has not committed to a branch yet. | ||
| // 3. [RuntimeATN.NextCompletionsFromSet] reports what may come next as a | ||
| // [CompletionInfo]: valid token types, plus [CompletionHint] entries that | ||
| // mark cross-reference positions so the provider can offer resolvable | ||
| // symbols instead of a bare identifier token. |
There was a problem hiding this comment.
Suggestion: Too in depth? I'm not sure this is interesting for adopters. They should never need to touch this or understand it beyond the basic workings of the parser (described above).
3 participants
