Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ST API: Missing support for workspace/didChangeWatchedFiles #892

Open
rchl opened this issue Feb 26, 2020 · 14 comments
Open

ST API: Missing support for workspace/didChangeWatchedFiles #892

rchl opened this issue Feb 26, 2020 · 14 comments
Labels
sublime issue Issues related to shortcomings or bugs in the ST API

Comments

@rchl
Copy link
Member

rchl commented Feb 26, 2020

  • OSX
  • LSP v0.9.5
  • LSP-Vue v0.0.14

Vue project with TS configuration intsconfig.json and custom types in types/index.d.ts.
Editing types in types/index.d.ts doesn't get reflected in *.vue file if I'm referring to those types there.
It works in plain *.js files (those pick up changes to types immediately).

My workaround is to trigger "LSP: Restart servers" command every time I change types.
No problem in VSCode.

@rchl rchl changed the title Diagnostic don't refresh in Vue files after types are changed Type changes not picked up in Vue files after types are changed Feb 26, 2020
@rchl
Copy link
Member Author

rchl commented Feb 26, 2020

I'll create test project and investigate more when I have time. Just wanted to log this as it bothered me for a while now.

@rwols
Copy link
Member

rwols commented Feb 26, 2020

My first instinct, without having used vue-lsp, is that we don't support workspace/didChangeWatchedFiles.

@rchl
Copy link
Member Author

rchl commented Feb 26, 2020

Yes, you are most likely right.

I've set-up a project with instructions at https://github.com/rchl/test-sublime-lsp-type-bug

When checking VSCode, I can see it triggering

[Trace - 9:55:53 PM] Sending notification 'workspace/didChangeWatchedFiles'.
Params: {
    "changes": [
        {
            "uri": "file:///Users/me/minimal/types/index.d.ts",
            "type": 2
        }
    ]
}

right after I make a change in types/index.d.ts.

The fact that the pure JS or TS file sees the changes immediately is probably because for that file LSP-vue is actually not used. Just a pure typescript server is, instead.

@rwols
Copy link
Member

rwols commented Feb 26, 2020

See: #516, and sublimehq/sublime_text#2669.

@rwols rwols added the sublime issue Issues related to shortcomings or bugs in the ST API label Feb 26, 2020
@rchl
Copy link
Member Author

rchl commented Feb 26, 2020

Extra note:
While server can request watcher to be registered using DidChangeWatchedFilesRegistrationOptions, this is not how Vetur (used by LSP-vue) does it at least (yet?).

Instead, its VSCode extension part (since vetur is composed of server and vscode extension) registers watcher at https://github.com/vuejs/vetur/blob/8b82c92a232086167af2969b42a6cc0218cf5c2e/client/client.ts#L53 .

So which files are to be watched is server-specific and so that would have to be decided per-server (in this case in LSP-vue package).

@rwols rwols changed the title Type changes not picked up in Vue files after types are changed ST API: Missing support for workspace/didChangeWatchedFiles Sep 1, 2020
@ayoub-benali
Copy link
Contributor

Does it make sense to have LSP provide a watcher API that language handlers can hook into ?
Because it is a similar missing features for Metals integration.

@rchl
Copy link
Member Author

rchl commented Jan 25, 2021

I don't think there is any good existing python library that we could use. I've implemented this feature using one of them but it was missing support for glob patterns which is required for LSP and wasn't using the most efficient file watching technique (on mac at least) so it introduced a significant overhead.

So if it'd be for me, I would probably go for node-based file watcher and a thin layer on top to provide an API.

Of course with node there is a problem that it's not always available on the system.

@ayoub-benali
Copy link
Contributor

Of course with node there is a problem that it's not always available on the system.

The alternative would be to embed a small binary with LSP ?

@rchl
Copy link
Member Author

rchl commented Jan 27, 2021

Not sure about the small part. The node binary is about 50~75MB per-platform.
If going that route, I'd rather fetch it at runtime than bundle with the package.

@ayoub-benali
Copy link
Contributor

no I wasn't talking about node but a dedicated binary utility that does only the file watching part

@rwols
Copy link
Member

rwols commented Jan 27, 2021

I fear that a dependency on a compiled binary would be a major pain to maintain. The package is pure-python now, so it automatically works on Apple Silicon (whether emulated darwin-x86_64 or native darwin-arm64), Nintendo Switch (linux-arm64, confirmed working), you name it.

@rwols
Copy link
Member

rwols commented Jan 27, 2021

Not only that, but Sublime Text already allocates a ton of file descriptors for its file watchers. So we would be halving the available file descriptors if we were to also track files.

EDIT: Well, maybe plugin_host gets a new set of file descriptors allocated because it's a separate process.

@rwols
Copy link
Member

rwols commented Jan 27, 2021

IMO I only see two options:

  1. We get an API. I learned some people experimented with this, but it'd send a large amount of data over the IPC layer which was apparently too costly. Maybe we can convince them that we only need "some" file notifications.
  2. SublimeHQ implements LSP natively and skips the trouble of the API layer

@rchl
Copy link
Member Author

rchl commented Nov 6, 2021

For the record, I've created https://packagecontrol.io/packages/LSP-file-watcher-chokidar which provides support for this notification in LSP until we'll get an API from ST that we could switch to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sublime issue Issues related to shortcomings or bugs in the ST API
Projects
None yet
Development

No branches or pull requests

3 participants