Conversation
- Store and track mixin filenames per URI in storage - Avoid reading files that are already cached in storage (opened by editor or loaded previously) - Only read newly added mixins during updates - Bump document refresh debounce to 500ms to reduce index jitter
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Reviewer's GuideIntroduces a debounced document refresh pipeline in the LSP server so indexing and mixin loading are coalesced per document, tracks previously loaded mixins to avoid redundant work, and ensures debouncer cleanup on server shutdown, with tests covering debounce behavior and stale index removal. Sequence diagram for debounced document refresh on changesequenceDiagram
actor Client
participant LSPServer as lspServer
participant Storage as storage
participant Debouncer as documentDebouncer
participant Index as index
participant Parser as parser
Client->>LSPServer: textDocumentDidChange(params)
LSPServer->>Storage: AddDocument(uri, text)
alt refresh debouncer exists
LSPServer->>Debouncer: Schedule(uri)
note over Debouncer: Reset or start timer for uri
note over Debouncer: After debounce delay
Debouncer->>LSPServer: refreshDocument(uri)
else no debouncer
LSPServer->>LSPServer: go refreshDocument(uri)
end
activate LSPServer
LSPServer->>Storage: GetDocument(uri)
alt document exists
LSPServer->>Index: IndexDocument(uri, *doc)
LSPServer->>LSPServer: loadMixins(uri)
LSPServer->>Storage: GetDocument(uri) for mixins
LSPServer->>Parser: getMixinFilenames(doc)
LSPServer->>Storage: GetMixins(uri)
LSPServer->>Storage: SetMixins(uri, currentMixins)
loop new mixin filenames only
LSPServer->>Storage: GetDocument(mixinURI)
alt mixin not yet managed
LSPServer->>Index: AddDocument for mixin
else already managed
LSPServer-->>LSPServer: skip mixin
end
end
else document missing
LSPServer-->>LSPServer: return
end
deactivate LSPServer
Sequence diagram for server shutdown cleaning debouncer timerssequenceDiagram
participant Server as lspServer
participant Debouncer as documentDebouncer
Server->>Server: shutdown(context)
alt refresh debouncer exists
Server->>Debouncer: Stop()
Debouncer->>Debouncer: stop all timers
end
Server->>Server: lsp.SetTraceValue(TraceValueOff)
Server-->>Server: return nil
Class diagram for debounced document refresh and storage mixin trackingclassDiagram
class lspServer {
-storage *storage
-parser *parser
-index *index
-refresh *documentDebouncer
-log commonlog_Logger
+initialized(context *glsp_Context, params *lsp_InitializedParams) error
+shutdown(context *glsp_Context) error
+loadMixins(uri string)
+refreshDocument(uri string)
+textDocumentDidOpen(context *glsp_Context, params *lsp_DidOpenTextDocumentParams) error
+textDocumentDidChange(context *glsp_Context, params *lsp_DidChangeTextDocumentParams) error
}
class storage {
-mu sync_RWMutex
-documents map_string_*string
-mixins map_string_[]string
+GetDocument(uri string) *string
+AddDocument(uri string, text string)
+SetMixins(uri string, mixins []string)
+GetMixins(uri string) []string
}
class documentDebouncer {
-delay time_Duration
-refresh func_string
-mu sync_Mutex
-timers map_string_*time_Timer
+Schedule(uri string)
+Stop()
-fire(uri string, timer *time_Timer)
}
class index {
+IndexDocument(uri string, text string)
}
class parser {
+getMixinFilenames(doc *string) []string
}
lspServer --> storage : uses
lspServer --> parser : uses
lspServer --> index : uses
lspServer --> documentDebouncer : owns refresh
documentDebouncer --> lspServer : calls refreshDocument
storage "1" --> "*" string : documents values
storage "1" --> "*" string : mixins entries
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="internal/lsp/debounce_test.go" line_range="13" />
<code_context>
+
+func TestDocumentDebouncerCoalescesRepeatedSchedules(t *testing.T) {
+ events := make(chan string, 2)
+ debouncer := newDocumentDebouncer(20*time.Millisecond, func(uri string) {
+ events <- uri
+ })
</code_context>
<issue_to_address>
**suggestion (testing):** Consider making the debounce timing assertions more robust to avoid flaky tests
These tests depend on tight, hard-coded timing (20ms delay with 60–300ms timeouts), which can be fragile on slower CI runners. To make them more resilient, consider: (1) increasing the timeout safety margin (e.g., 5–10× the debounce delay), and/or (2) extracting the timing values into constants so they can be adjusted centrally if flakes occur.
Suggested implementation:
```golang
import (
"testing"
"time"
"github.com/tliron/glsp"
lsp "github.com/tliron/glsp/protocol_3_16"
)
const (
// testDebounceDelay is the debounce delay used for tests.
testDebounceDelay = 20 * time.Millisecond
// testDebounceTimeoutShort is a generous timeout (10× the debounce delay)
// used for assertions that a debounced event *should* have fired.
testDebounceTimeoutShort = 10 * testDebounceDelay
// testDebounceTimeoutLong is a longer timeout used where the test logic
// needs more time; keeping it a named constant allows easy adjustment
// if CI flakes occur.
testDebounceTimeoutLong = 15 * testDebounceDelay
)
```
```golang
func TestDocumentDebouncerCoalescesRepeatedSchedules(t *testing.T) {
events := make(chan string, 2)
debouncer := newDocumentDebouncer(testDebounceDelay, func(uri string) {
events <- uri
})
defer debouncer.Stop()
```
I only see part of the file. To fully apply the suggestion, you should:
1. Replace any hard-coded timeouts like `time.After(60 * time.Millisecond)` with `time.After(testDebounceTimeoutShort)`.
2. Replace any longer hard-coded timeouts such as `time.After(300 * time.Millisecond)` (or similar) with `time.After(testDebounceTimeoutLong)` or introduce additional constants if you need more distinct timeout categories.
3. Replace any other `newDocumentDebouncer(<literal> * time.Millisecond, ...)` calls with `newDocumentDebouncer(testDebounceDelay, ...)` unless a test intentionally uses a different delay.
This will centralize timing configuration and make it easy to tweak if you ever see CI flakes.
</issue_to_address>
### Comment 2
<location path="internal/lsp/debounce_test.go" line_range="37" />
<code_context>
+ }
+}
+
+func TestTextDocumentDidChangeUsesLatestDocumentAfterDebounce(t *testing.T) {
+ server := &lspServer{
+ storage: newStorage(),
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test that verifies no further refreshes occur after the debouncer is stopped (e.g. during shutdown)
Given the new `Stop()` on `documentDebouncer` and its use during `shutdown`, please also add a test that:
- schedules a refresh,
- calls `Stop()` (or simulates `shutdown`),
- waits longer than the debounce delay,
- and asserts the refresh callback is never invoked.
This will verify that in-flight timers are cancelled and no index updates occur after shutdown.
</issue_to_address>
### Comment 3
<location path="internal/lsp/debounce_test.go" line_range="38" />
<code_context>
+}
+
+func TestTextDocumentDidChangeUsesLatestDocumentAfterDebounce(t *testing.T) {
+ server := &lspServer{
+ storage: newStorage(),
+ parser: newParser(logger),
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests that cover mixin-related behavior with the new debounced refresh flow
This test covers `textDocumentDidChange` focusing on command indexing, but the new `refreshDocument` flow also calls `loadMixins`, which now tracks previous vs current mixins and skips already-managed documents. Please add an integration-style test that changes a document’s mixins over time and asserts that:
- newly added mixins are loaded,
- previously used mixins are not redundantly reloaded, and
- mixins already in storage are skipped.
This will directly exercise the optimized index/mixin update path and guard against regressions in mixin handling.
Suggested implementation:
```golang
}
}
type trackingStorage struct {
Storage interface {
// Adjust these methods to match the actual storage interface.
// The important part is that this wrapper is used by lspServer so we can
// count how often mixin documents are loaded.
SaveDocument(ctx context.Context, uri lsp.DocumentURI, text string) error
LoadDocument(ctx context.Context, uri lsp.DocumentURI) (string, error)
}
loadCounts map[lsp.DocumentURI]int
}
func newTrackingStorage(underlying interface {
SaveDocument(ctx context.Context, uri lsp.DocumentURI, text string) error
LoadDocument(ctx context.Context, uri lsp.DocumentURI) (string, error)
}) *trackingStorage {
return &trackingStorage{
Storage: underlying,
loadCounts: make(map[lsp.DocumentURI]int),
}
}
func (s *trackingStorage) SaveDocument(ctx context.Context, uri lsp.DocumentURI, text string) error {
// Pass-through; we don't need to track writes for this test.
return s.Storage.SaveDocument(ctx, uri, text)
}
func (s *trackingStorage) LoadDocument(ctx context.Context, uri lsp.DocumentURI) (string, error) {
s.loadCounts[uri]++
return s.Storage.LoadDocument(ctx, uri)
}
func TestRefreshDocumentDebouncedMixins(t *testing.T) {
logger := logger // reuse the package-level test logger if available
baseStorage := newStorage()
tracking := newTrackingStorage(baseStorage)
server := &lspServer{
// NOTE: lspServer.storage must be compatible with trackingStorage's interface.
storage: tracking,
parser: newParser(logger),
index: newIndex(logger),
log: logger,
}
server.refresh = newDocumentDebouncer(20*time.Millisecond, server.refreshDocument)
defer server.refresh.Stop()
ctx := context.Background()
mainURI := lsp.DocumentURI("file:///tmp/lets.yaml")
mixin1 := lsp.DocumentURI("file:///tmp/mixin1.lets")
mixin2 := lsp.DocumentURI("file:///tmp/mixin2.lets")
mixin3 := lsp.DocumentURI("file:///tmp/mixin3.lets")
// Helper to trigger a debounced refresh via textDocument/didChange using the
// same entrypoint as the other tests.
changeMain := func(text string) {
params := &lsp.DidChangeTextDocumentParams{
TextDocument: lsp.VersionedTextDocumentIdentifier{
TextDocumentIdentifier: lsp.TextDocumentIdentifier{URI: mainURI},
},
ContentChanges: []lsp.TextDocumentContentChangeEvent{
{Text: text},
},
}
if err := server.textDocumentDidChange(ctx, params); err != nil {
t.Fatalf("textDocumentDidChange: %v", err)
}
time.Sleep(50 * time.Millisecond) // wait for debounce to fire
}
// 1) Start with a document that uses mixin1.
changeMain("uses:\n - " + string(mixin1) + "\n")
if got := tracking.loadCounts[mixin1]; got != 1 {
t.Fatalf("expected mixin1 to be loaded once on initial refresh, got %d", got)
}
if got := tracking.loadCounts[mixin2]; got != 0 {
t.Fatalf("expected mixin2 to not be loaded yet, got %d", got)
}
if got := tracking.loadCounts[mixin3]; got != 0 {
t.Fatalf("expected mixin3 to not be loaded yet, got %d", got)
}
// 2) Change the document to still use mixin1 and also use mixin2.
changeMain("uses:\n - " + string(mixin1) + "\n - " + string(mixin2) + "\n")
// mixin1 is still in use but should not be redundantly reloaded.
if got := tracking.loadCounts[mixin1]; got != 1 {
t.Fatalf("expected mixin1 to not be reloaded, still 1 load; got %d", got)
}
// mixin2 is newly added and should be loaded exactly once.
if got := tracking.loadCounts[mixin2]; got != 1 {
t.Fatalf("expected mixin2 to be loaded once after being added, got %d", got)
}
// 3) Pre-load mixin3 into storage to simulate it already being managed.
if err := tracking.SaveDocument(ctx, mixin3, "command: mixin3"); err != nil {
t.Fatalf("preloading mixin3 into storage: %v", err)
}
// Now change the document to use mixin1, mixin2, and mixin3.
changeMain("uses:\n - " + string(mixin1) + "\n - " + string(mixin2) + "\n - " + string(mixin3) + "\n")
// mixin1 and mixin2 are still in use; no extra loads.
if got := tracking.loadCounts[mixin1]; got != 1 {
t.Fatalf("expected mixin1 to still have exactly 1 load, got %d", got)
}
if got := tracking.loadCounts[mixin2]; got != 1 {
t.Fatalf("expected mixin2 to still have exactly 1 load, got %d", got)
}
// mixin3 was already present in storage and should be skipped by loadMixins.
if got := tracking.loadCounts[mixin3]; got != 0 {
t.Fatalf("expected mixin3 to be skipped because it is already in storage, got %d loads", got)
}
}
func TestTextDocumentDidChangeUsesLatestDocumentAfterDebounce(t *testing.T) {
server := &lspServer{
storage: newStorage(),
parser: newParser(logger),
index: newIndex(logger),
log: logger,
}
server.refresh = newDocumentDebouncer(20*time.Millisecond, server.refreshDocument)
defer server.refresh.Stop()
params := &lsp.DidChangeTextDocumentParams{
TextDocument: lsp.VersionedTextDocumentIdentifier{
TextDocumentIdentifier: lsp.TextDocumentIdentifier{URI: "file:///tmp/lets.yaml"},
},
```
The above changes assume certain shapes of the storage and server APIs that may differ slightly from your actual code:
1. **storage abstraction**
- Replace the `trackingStorage` interface methods (`SaveDocument`, `LoadDocument`) with the actual methods your `lspServer` calls during `refreshDocument` / `loadMixins`.
- If your concrete storage type is exported (e.g. `type storage struct { ... }`), make `trackingStorage` embed it (e.g. `type trackingStorage struct { *storage; loadCounts map[...]int }`) and override only the load method that `loadMixins` uses to fetch mixin documents.
2. **URIs and mixin syntax**
- Adjust the `mixin1`, `mixin2`, and `mixin3` URIs and the `changeMain` text payloads to match how your parser detects mixins (e.g. different YAML keys, relative paths, etc.). The test is structured so that:
- first change → mixin1 only,
- second change → mixin1 + mixin2,
- third change → mixin1 + mixin2 + mixin3 (already in storage).
3. **Debouncer triggering**
- Replace the `time.Sleep(50 * time.Millisecond)` with the same synchronization strategy used by your existing debounce tests (e.g. reading from a channel of refresh events if the debouncer exposes one, or using a slightly longer sleep consistent with your other tests).
4. **Error handling**
- If `textDocumentDidChange` has a different signature (e.g. doesn’t return an error), remove the error check accordingly.
Once you align `trackingStorage` with your actual storage API and tweak the URI/mixin syntax to what your parser expects, this test will directly exercise the debounced `refreshDocument` + `loadMixins` path and verify that:
- new mixins are loaded,
- existing-but-still-used mixins are not redundantly reloaded, and
- already-stored mixins are skipped.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| func TestDocumentDebouncerCoalescesRepeatedSchedules(t *testing.T) { | ||
| events := make(chan string, 2) | ||
| debouncer := newDocumentDebouncer(20*time.Millisecond, func(uri string) { |
There was a problem hiding this comment.
suggestion (testing): Consider making the debounce timing assertions more robust to avoid flaky tests
These tests depend on tight, hard-coded timing (20ms delay with 60–300ms timeouts), which can be fragile on slower CI runners. To make them more resilient, consider: (1) increasing the timeout safety margin (e.g., 5–10× the debounce delay), and/or (2) extracting the timing values into constants so they can be adjusted centrally if flakes occur.
Suggested implementation:
import (
"testing"
"time"
"github.com/tliron/glsp"
lsp "github.com/tliron/glsp/protocol_3_16"
)
const (
// testDebounceDelay is the debounce delay used for tests.
testDebounceDelay = 20 * time.Millisecond
// testDebounceTimeoutShort is a generous timeout (10× the debounce delay)
// used for assertions that a debounced event *should* have fired.
testDebounceTimeoutShort = 10 * testDebounceDelay
// testDebounceTimeoutLong is a longer timeout used where the test logic
// needs more time; keeping it a named constant allows easy adjustment
// if CI flakes occur.
testDebounceTimeoutLong = 15 * testDebounceDelay
)func TestDocumentDebouncerCoalescesRepeatedSchedules(t *testing.T) {
events := make(chan string, 2)
debouncer := newDocumentDebouncer(testDebounceDelay, func(uri string) {
events <- uri
})
defer debouncer.Stop()I only see part of the file. To fully apply the suggestion, you should:
- Replace any hard-coded timeouts like
time.After(60 * time.Millisecond)withtime.After(testDebounceTimeoutShort). - Replace any longer hard-coded timeouts such as
time.After(300 * time.Millisecond)(or similar) withtime.After(testDebounceTimeoutLong)or introduce additional constants if you need more distinct timeout categories. - Replace any other
newDocumentDebouncer(<literal> * time.Millisecond, ...)calls withnewDocumentDebouncer(testDebounceDelay, ...)unless a test intentionally uses a different delay.
This will centralize timing configuration and make it easy to tweak if you ever see CI flakes.
| } | ||
| } | ||
|
|
||
| func TestTextDocumentDidChangeUsesLatestDocumentAfterDebounce(t *testing.T) { |
There was a problem hiding this comment.
suggestion (testing): Add a test that verifies no further refreshes occur after the debouncer is stopped (e.g. during shutdown)
Given the new Stop() on documentDebouncer and its use during shutdown, please also add a test that:
- schedules a refresh,
- calls
Stop()(or simulatesshutdown), - waits longer than the debounce delay,
- and asserts the refresh callback is never invoked.
This will verify that in-flight timers are cancelled and no index updates occur after shutdown.
| } | ||
|
|
||
| func TestTextDocumentDidChangeUsesLatestDocumentAfterDebounce(t *testing.T) { | ||
| server := &lspServer{ |
There was a problem hiding this comment.
suggestion (testing): Add tests that cover mixin-related behavior with the new debounced refresh flow
This test covers textDocumentDidChange focusing on command indexing, but the new refreshDocument flow also calls loadMixins, which now tracks previous vs current mixins and skips already-managed documents. Please add an integration-style test that changes a document’s mixins over time and asserts that:
- newly added mixins are loaded,
- previously used mixins are not redundantly reloaded, and
- mixins already in storage are skipped.
This will directly exercise the optimized index/mixin update path and guard against regressions in mixin handling.
Suggested implementation:
}
}
type trackingStorage struct {
Storage interface {
// Adjust these methods to match the actual storage interface.
// The important part is that this wrapper is used by lspServer so we can
// count how often mixin documents are loaded.
SaveDocument(ctx context.Context, uri lsp.DocumentURI, text string) error
LoadDocument(ctx context.Context, uri lsp.DocumentURI) (string, error)
}
loadCounts map[lsp.DocumentURI]int
}
func newTrackingStorage(underlying interface {
SaveDocument(ctx context.Context, uri lsp.DocumentURI, text string) error
LoadDocument(ctx context.Context, uri lsp.DocumentURI) (string, error)
}) *trackingStorage {
return &trackingStorage{
Storage: underlying,
loadCounts: make(map[lsp.DocumentURI]int),
}
}
func (s *trackingStorage) SaveDocument(ctx context.Context, uri lsp.DocumentURI, text string) error {
// Pass-through; we don't need to track writes for this test.
return s.Storage.SaveDocument(ctx, uri, text)
}
func (s *trackingStorage) LoadDocument(ctx context.Context, uri lsp.DocumentURI) (string, error) {
s.loadCounts[uri]++
return s.Storage.LoadDocument(ctx, uri)
}
func TestRefreshDocumentDebouncedMixins(t *testing.T) {
logger := logger // reuse the package-level test logger if available
baseStorage := newStorage()
tracking := newTrackingStorage(baseStorage)
server := &lspServer{
// NOTE: lspServer.storage must be compatible with trackingStorage's interface.
storage: tracking,
parser: newParser(logger),
index: newIndex(logger),
log: logger,
}
server.refresh = newDocumentDebouncer(20*time.Millisecond, server.refreshDocument)
defer server.refresh.Stop()
ctx := context.Background()
mainURI := lsp.DocumentURI("file:///tmp/lets.yaml")
mixin1 := lsp.DocumentURI("file:///tmp/mixin1.lets")
mixin2 := lsp.DocumentURI("file:///tmp/mixin2.lets")
mixin3 := lsp.DocumentURI("file:///tmp/mixin3.lets")
// Helper to trigger a debounced refresh via textDocument/didChange using the
// same entrypoint as the other tests.
changeMain := func(text string) {
params := &lsp.DidChangeTextDocumentParams{
TextDocument: lsp.VersionedTextDocumentIdentifier{
TextDocumentIdentifier: lsp.TextDocumentIdentifier{URI: mainURI},
},
ContentChanges: []lsp.TextDocumentContentChangeEvent{
{Text: text},
},
}
if err := server.textDocumentDidChange(ctx, params); err != nil {
t.Fatalf("textDocumentDidChange: %v", err)
}
time.Sleep(50 * time.Millisecond) // wait for debounce to fire
}
// 1) Start with a document that uses mixin1.
changeMain("uses:\n - " + string(mixin1) + "\n")
if got := tracking.loadCounts[mixin1]; got != 1 {
t.Fatalf("expected mixin1 to be loaded once on initial refresh, got %d", got)
}
if got := tracking.loadCounts[mixin2]; got != 0 {
t.Fatalf("expected mixin2 to not be loaded yet, got %d", got)
}
if got := tracking.loadCounts[mixin3]; got != 0 {
t.Fatalf("expected mixin3 to not be loaded yet, got %d", got)
}
// 2) Change the document to still use mixin1 and also use mixin2.
changeMain("uses:\n - " + string(mixin1) + "\n - " + string(mixin2) + "\n")
// mixin1 is still in use but should not be redundantly reloaded.
if got := tracking.loadCounts[mixin1]; got != 1 {
t.Fatalf("expected mixin1 to not be reloaded, still 1 load; got %d", got)
}
// mixin2 is newly added and should be loaded exactly once.
if got := tracking.loadCounts[mixin2]; got != 1 {
t.Fatalf("expected mixin2 to be loaded once after being added, got %d", got)
}
// 3) Pre-load mixin3 into storage to simulate it already being managed.
if err := tracking.SaveDocument(ctx, mixin3, "command: mixin3"); err != nil {
t.Fatalf("preloading mixin3 into storage: %v", err)
}
// Now change the document to use mixin1, mixin2, and mixin3.
changeMain("uses:\n - " + string(mixin1) + "\n - " + string(mixin2) + "\n - " + string(mixin3) + "\n")
// mixin1 and mixin2 are still in use; no extra loads.
if got := tracking.loadCounts[mixin1]; got != 1 {
t.Fatalf("expected mixin1 to still have exactly 1 load, got %d", got)
}
if got := tracking.loadCounts[mixin2]; got != 1 {
t.Fatalf("expected mixin2 to still have exactly 1 load, got %d", got)
}
// mixin3 was already present in storage and should be skipped by loadMixins.
if got := tracking.loadCounts[mixin3]; got != 0 {
t.Fatalf("expected mixin3 to be skipped because it is already in storage, got %d loads", got)
}
}
func TestTextDocumentDidChangeUsesLatestDocumentAfterDebounce(t *testing.T) {
server := &lspServer{
storage: newStorage(),
parser: newParser(logger),
index: newIndex(logger),
log: logger,
}
server.refresh = newDocumentDebouncer(20*time.Millisecond, server.refreshDocument)
defer server.refresh.Stop()
params := &lsp.DidChangeTextDocumentParams{
TextDocument: lsp.VersionedTextDocumentIdentifier{
TextDocumentIdentifier: lsp.TextDocumentIdentifier{URI: "file:///tmp/lets.yaml"},
},The above changes assume certain shapes of the storage and server APIs that may differ slightly from your actual code:
-
storage abstraction
- Replace the
trackingStorageinterface methods (SaveDocument,LoadDocument) with the actual methods yourlspServercalls duringrefreshDocument/loadMixins. - If your concrete storage type is exported (e.g.
type storage struct { ... }), maketrackingStorageembed it (e.g.type trackingStorage struct { *storage; loadCounts map[...]int }) and override only the load method thatloadMixinsuses to fetch mixin documents.
- Replace the
-
URIs and mixin syntax
- Adjust the
mixin1,mixin2, andmixin3URIs and thechangeMaintext payloads to match how your parser detects mixins (e.g. different YAML keys, relative paths, etc.). The test is structured so that:- first change → mixin1 only,
- second change → mixin1 + mixin2,
- third change → mixin1 + mixin2 + mixin3 (already in storage).
- Adjust the
-
Debouncer triggering
- Replace the
time.Sleep(50 * time.Millisecond)with the same synchronization strategy used by your existing debounce tests (e.g. reading from a channel of refresh events if the debouncer exposes one, or using a slightly longer sleep consistent with your other tests).
- Replace the
-
Error handling
- If
textDocumentDidChangehas a different signature (e.g. doesn’t return an error), remove the error check accordingly.
- If
Once you align trackingStorage with your actual storage API and tweak the URI/mixin syntax to what your parser expects, this test will directly exercise the debounced refreshDocument + loadMixins path and verify that:
- new mixins are loaded,
- existing-but-still-used mixins are not redundantly reloaded, and
- already-stored mixins are skipped.
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.
Summary by Sourcery
Debounce and centralize document refresh operations in the LSP server to reduce redundant indexing and mixin loading, while tracking mixin state per document.
Enhancements:
Tests: