Send CompletionContext so servers return trigger-character completions#193
Send CompletionContext so servers return trigger-character completions#193
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves LSP completion results when invoking completion immediately after a server-defined trigger character (e.g., window.) by sending CompletionContext so servers can return member completions in that scenario.
Changes:
- Extend
LSP::Client#completionto accept and send an optionalcontextparameter. - Update
lsp_completionto detect trigger characters fromcompletionProvider.triggerCharactersand sendtriggerKind=2+triggerCharacterwhen appropriate; otherwise sendtriggerKind=1.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/textbringer/lsp/client.rb | Adds optional context to the completion request payload. |
| lib/textbringer/commands/lsp.rb | Computes CompletionContext based on trigger characters and passes it through to the client completion request. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/textbringer/commands/lsp.rb
Outdated
| context = if prefix.empty? && trigger_chars.include?(char_before_start) | ||
| { triggerKind: 2, triggerCharacter: char_before_start } | ||
| else | ||
| { triggerKind: 1 } | ||
| end |
There was a problem hiding this comment.
triggerKind is currently set using bare integers (1/2). Consider introducing named constants or at least inline comments (as done for signature help below) so it's clear these correspond to LSP CompletionTriggerKind::Invoked and ::TriggerCharacter and to avoid magic numbers.
| # Determine trigger context: if the character immediately before start_point | ||
| # is a completion trigger character, inform the server so it returns | ||
| # member completions (e.g. after "."). | ||
| trigger_chars = client.server_capabilities | ||
| .dig("completionProvider", "triggerCharacters") || [] | ||
| char_before_start = buffer.save_point { | ||
| buffer.goto_char(start_point) | ||
| buffer.point > 0 ? (buffer.backward_char; buffer.char_after) : nil | ||
| } | ||
| context = if prefix.empty? && trigger_chars.include?(char_before_start) | ||
| { triggerKind: 2, triggerCharacter: char_before_start } | ||
| else | ||
| { triggerKind: 1 } | ||
| end | ||
|
|
||
| # Request completion | ||
| client.completion(uri: uri, line: line, character: character) do |items, error| | ||
| client.completion(uri: uri, line: line, character: character, context: context) do |items, error| |
There was a problem hiding this comment.
The new trigger-context selection logic (prefix empty + previous character is in completionProvider.triggerCharacters => triggerKind: 2) is behavioral and easy to regress. Please add tests that exercise both branches (trigger-character vs invoked), ideally stubbing a client with server_capabilities and asserting the context sent to client.completion.
| def completion(uri:, line:, character:, context: nil, &callback) | ||
| return unless @initialized | ||
|
|
||
| params = { | ||
| textDocument: { uri: uri }, | ||
| position: { line: line, character: character } | ||
| } | ||
| params[:context] = context if context |
There was a problem hiding this comment.
completion now supports sending CompletionContext (via the new context: kwarg), but the client capabilities sent during initialize don't advertise textDocument.completion.contextSupport. Some LSP servers gate their use of completion context on this capability, so this change may not take effect (or may behave inconsistently) unless contextSupport: true is added under textDocument: { completion: ... } in client_capabilities (and the corresponding capability test updated).
| def completion(uri:, line:, character:, context: nil, &callback) | |
| return unless @initialized | |
| params = { | |
| textDocument: { uri: uri }, | |
| position: { line: line, character: character } | |
| } | |
| params[:context] = context if context | |
| def completion(uri:, line:, character:, &callback) | |
| return unless @initialized | |
| params = { | |
| textDocument: { uri: uri }, | |
| position: { line: line, character: character } | |
| } |
Without a context, some LSP servers don't return member completions when the cursor is right after a trigger character (e.g. "window."). Now lsp_completion checks completionProvider.triggerCharacters and: - sends triggerKind=2 + triggerCharacter when prefix is empty and the character before start_point is a trigger character (e.g. ".") - sends triggerKind=1 (Invoked) otherwise Using triggerKind=2 only when prefix is empty avoids returning the full unfiltered member list while the user is mid-identifier. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add inline comments (Invoked / TriggerCharacter) to triggerKind values - Extract context-building into lsp_completion_context helper method - Add contextSupport: true to textDocument.completion client capabilities - Add test_client_capabilities assertion for completion contextSupport - Add test/textbringer/commands/test_lsp.rb with 6 tests covering both branches of the trigger context selection logic Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
shugo
force-pushed
the
fix/lsp-completion-trigger-context
branch
from
f81f766 to
8b281c5
Compare
Without a context, some LSP servers don't return member completions when the cursor is right after a trigger character (e.g. "window."). Now lsp_completion checks completionProvider.triggerCharacters and:
Using triggerKind=2 only when prefix is empty avoids returning the full unfiltered member list while the user is mid-identifier.