[ty] Add basic file watching to server#17912
Conversation
MichaReiser
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
MichaReiser
added
server
|
| serde_json::to_value(DidChangeWatchedFilesRegistrationOptions { | ||
| watchers: vec
There was a problem hiding this comment.
Mostly curious: does this mean that we're reacting to changes that match the **/ty.toml pattern. Or that we actively scan the whole project for this pattern when starting the server (which could be quite slow?)
There was a problem hiding this comment.
The VS code file watcher sends us notifications if any file inside the project matching ty.toml gets changed.
| FileSystemWatcher { | ||
| glob_pattern: lsp_types::GlobPattern::String( | ||
| "**/.gitignore".into(), | ||
| ), | ||
| kind: None, | ||
| }, | ||
| FileSystemWatcher { | ||
| glob_pattern: lsp_types::GlobPattern::String("**/.ignore".into()), | ||
| kind: None, | ||
| }, |
There was a problem hiding this comment.
If we want to be (more) exhaustive here, we would probably also have to add .git/info/exclude?
There was a problem hiding this comment.
Maybe. It's not something that we currently handle in apply_changes. (but maybe should?)
There was a problem hiding this comment.
I'll ignore this for now. There's another issue around better handling for git ignored files
| } | ||
| }; | ||
|
|
||
| by_db |
There was a problem hiding this comment.
Is this suppose to be ty_db ?
There was a problem hiding this comment.
Oh, nevermind, I missed the definition. I think it would be useful to include something before ??_by_db
MichaReiser
changed the title
MichaReiser
force-pushed
the
micha/server-watch
branch
from
1b606d0 to
5d17f87
Compare
…lass * origin/main: [`pylint`] add fix safety section (`PLC2801`) (#17825) Add instructions on how to upgrade to a newer Rust version (#17928) [parser] Flag single unparenthesized generator expr with trailing comma in arguments. (#17893) [ty] Ensure that `T` is disjoint from `~T` even when `T` is a TypeVar (#17922) [ty] Sort collected diagnostics before snapshotting them in mdtest (#17926) [ty] Add basic file watching to server (#17912) Make completions an opt-in LSP feature (#17921) Add link to `ty` issue tracker (#17924) [ty] Add support for `__all__` (#17856) [ty] fix assigning a typevar to a union with itself (#17910) [ty] Improve UX for `[duplicate-base]` diagnostics (#17914) Clean up some Ruff references in the ty server (#17920)
3 participants
Summary
This PR does the absolut minimum to add file watching support to ty's LSP.
The main limitation is that it only watches file in the project directory. It doesn't watch for
changes to:
It also has no special handling in case the user adds or removes a
ty.tomlorpyproject.tomlwhich would change the project's root directory (other than what ty'sapply_changesprovides by default). The solutionhere is a tighter integration (or at least code sharing) with
ProjectWatcher. Ideally, the CLI and LSP would share all code and only differ by the file watching backend.This gives us:
Test Plan
I edited a file outside of VS code and verified that diagnostics and inlay hints are correctly recomputed.
I verified that changing the python version correctly propagates to inlays.