Watch files over LSP #806
Watch files over LSP #806
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for watching files over LSP by integrating file watcher functionality into various parts of the project and refactoring how file events are processed.
- Introduces new file watch APIs and methods in the project and LSP server modules.
- Refactors and propagates comparePathsOptions, file watchers, and diagnostic publishing across services and tests.
- Updates tests and utilities for consistent use of configuration and file watching.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tsoptions/* | Introduces new utility functions for pattern matching. |
| internal/project/* | Adds file watcher fields and update logic in projects. |
| internal/lsp/server.go | Integrates LSP file watch requests, notifications, and dynamic registration. |
| internal/api/api.go | Updates to script info creation reflecting FS injection. |
| internal/testutil/projecttestutil/projecttestutil.go | Refactors file variable naming to improve consistency. |
Comments suppressed due to low confidence (1)
internal/lsp/server.go:70
- The variable name 'watcheEnabled' appears to have a typo. Consider renaming it to 'watchEnabled' for clarity and consistency.
watcheEnabled bool
| return false | ||
| } | ||
|
|
||
| if slices.ContainsFunc(p.ConfigFile.configFileSpecs.validatedFilesSpec, func(f string) bool { |
There was a problem hiding this comment.
You also want to check if extension is valid to avoid matching js files when they may not be included https://github.com/microsoft/TypeScript/blob/main/src/server/editorServices.ts#L4494
| } | ||
|
|
||
| func (p *Project) getModuleResolutionWatchGlobs() (failedLookups map[tspath.Path]string, affectingLocaions map[tspath.Path]string) { | ||
| failedLookups = make(map[tspath.Path]string) |
There was a problem hiding this comment.
This may be ok for initial implementation but iterating over so many resolutions and so many failed lookups has been perf bottleneck. It was also memory issue for bigger code bases to store all the failed lookups as is and not watch the directories instead
| } else if updated { | ||
| p.log("Affecting location watches updated:\n" + formatFileList(p.affectingLocationsWatch.globs, "\t", hr)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Need missing file watches .. On that note currently we add root files that are "present" but we need to be adding all root files and let program handle it so that it can issue "missing file" diagnostic and we can add watch as well
| path := p.toPath(fileName) | ||
| if p.kind == KindConfigured { | ||
| if p.rootFileNames.Has(path) || p.parsedCommandLine.MatchesFileName(fileName, p.comparePathsOptions) { | ||
| p.pendingConfigReload = true |
There was a problem hiding this comment.
For now this is ok but it is unnecessary to reparse the "config file" and calculate everything if config file has not changed. Just getting new list of root file names is pattern matching and is usually faster.
|
|
||
| w.globs = newGlobs | ||
| if w.watcherID != "" { | ||
| if err = w.client.UnwatchFiles(w.watcherID); err != nil { |
There was a problem hiding this comment.
Fyi. Removing watches and then adding again for unchanged file list in past has been pretty costly from perf perspective.
This adds LSP file watching requests for wildcard directories, failed/affecting lookup locations, and tsconfig files.