UnWatch for rascal file systems is not working

[DavyLandman]

Watch of the RascalFileSystemProvider is un-finished (contains a TODO called "fix this"). It's not correctly registering events an unwatching. On the java side the unwatch is not implemented and the watch is incomplete.

[jurgenvinju]

Which code/files is this about?

[DavyLandman]

It's the rascal resolvers exposed into vscode direction. (The RascalFileSystemProvider)

[jurgenvinju]

This also has an effect on the reloading of modules in the REPL; if the project is configured with project://projectName it does not work, while if it is configured with file:///path/to/project it does work.

[jurgenvinju]

I confirmed that watchers over rascal-only-vfs schemes work, by doing this:

• creating files in a rascal-based file systems like tmp:///, home:/// and project:///
• opening editors for these URIs using both links in the editor and util::IDEServices::edit
• editing with vi the same files outside of the control of VScode
• seeing the contents change immediately in the editor.

In my head, this means that the watch functionality over our VFS bridge works. Right?

[DavyLandman]

The unwatch function is not implemented in IRascalFileSystemServices.

[jurgenvinju]

Notes to self; there is something of an "impedence mismatch" here;

• unwatch is used only to generate a "disposer" function on the VScode side.
• unwatch on the Rascal side requires parameters recursive, uri, and the exact callback function to unregister.
• unwatch on the VScode side does not have a callback associated with it, and neither does watch. The watcher interface works with more general event listeners.
• still, multiple calls to watch would lead to multiple events, so we can't simpy remove all the watchers with the same URI. or can we?
• and, we still have not know how to implement recursive for unwatch.

[jurgenvinju]

• the design of unwatch is spread over usethesource/rascal-language-servers and usethesource/rascal.
• there is no code in libraries I know of that uses IO::watch and IO::unwatch. That makes it hard to decide if we can change something for the benefit of rascal-language-servers without doing harm.

[jurgenvinju]

Will look at this after the release; any kind of fix seems to be rather invasive.

[DavyLandman]

I will say that the recursive thing is very messy. Since there is no way for a scheme to say: hey I can handle a recursive watch, so IURIResolverRegistry always iterates the whole directory and registers watches for every subdir. There are 2 things with this I don't like:

1. for large directories it's slow since it iterates the file system and it registers a lot of watches, even though file backed ones have native recursive watches available.
2. If a new directory is added, you get an event about it, but that new directory is then not watched, as we didn't request a native recursive watch.

So changing the APIs a bit might be needed anyway.

unwatch is used only to generate a "disposer" function on the VScode side.

This happens more than you might think. Almost everything is disposable in VS Code. In a way, a disposable thing is a their way to say: hey, this can get cancelled. So if a user closes a file, you might get a dispose call of the watcher that was opened for this file. So unwatch is very much a needed api, als see #209 where you get this message coming around in your output log.

unwatch on the VScode side does not have a callback associated with it, and neither does watch. The watcher interface works with more general event listeners.

I think what is happening is that in one direction we are translating VS Code VFS to rascal VFS. And in the other direction we are translating Rascal VFS to VS Code VFS. This duality is the cause for #160. Are you talking about that?

[jurgenvinju]

Note: native watchers can not watch individual files. They all watch entire folders always. We filter the spurious events for file-specific watchers on the way back.

And: The recursion we do for the recursive flag is only used for schemes that do not implement a native watcher. Native watchers are expected to implement the recursive flag themselves, always. Don't know a better way to implement this for the other schemes, but I'm open to suggestions. Definitely the native watchers are always better.

I know unwatch is used a lot, that's why we have to fix this bug. The note was to say there is currently only one use of it to consider if we need to redesign it on the LSP side.

The final remark on the APIs is the actual mismatch that we need a solution for. Watchers in Rascal are removed on the identity of the callback function. There is no such identity to match on the VScode side, yet. I'm starting to think that VScode can/will only register one watcher per URI anyway, so we might use the URI twice and also identify the callback with it, or make all callbacks equal regardless such that there is only one callback per URI, and unwatch can remove it from the set.

This assumption is based on reading the event code in VScode but I've to confirm/deny this before we move on.

On Mon, 15 May 2023 at 20:07, Davy Landman @.***> wrote:

I will say that the recursive thing is very messy. Since there is no way for a scheme to say: hey I can handle a recursive watch, so IURIResolverRegistry always iterates the whole directory and registers watches for every subdir. There are 2 things with this I don't like:

1. for large directories it's slow since it iterates the file system and it registers a lot of watches, even though file backed ones have native recursive watches available.
2. If a new directory is added, you get an event about it, but that new directory is then not watched, as we didn't request a native recursive watch.

So changing the APIs a bit might be needed anyway.

unwatch is used only to generate a "disposer" function on the VScode side.

This happens more than you might think. Almost everything is disposable in VS Code. In a way, a disposable thing is a their way to say: hey, this can get cancelled. So if a user closes a file, you might get a dispose call of the watcher that was opened for this file. So unwatch is very much a needed api, als see #209 https://github.com/usethesource/rascal-language-servers/issues/209 where you get this message coming around in your output log.

unwatch on the VScode side does not have a callback associated with it, and neither does watch. The watcher interface works with more general event listeners.

I think what is happening is that in one direction we are translating VS Code VFS to rascal VFS. And in the other direction we are translating Rascal VFS to VS Code VFS. This duality is the cause for #160 https://github.com/usethesource/rascal-language-servers/issues/160. Are you talking about that?

— Reply to this email directly, view it on GitHub https://github.com/usethesource/rascal-language-servers/issues/159#issuecomment-1548323908, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPF5F46B3BSQNBB6GNFAZTXGJWHZANCNFSM5WBXMOQA . You are receiving this because you commented.Message ID: @.***>

-- Jurgen Vinju http://jurgen.vinju.org CWI SWAT TU Eindhoven Swat.engineering BV

[DavyLandman]

And: The recursion we do for the recursive flag is only used for schemes that do not implement a native watcher. Native watchers are expected to implement the recursive flag themselves, always. Don't know a better way to implement this for the other schemes, but I'm open to suggestions. Definitely the native watchers are always better.

https://github.com/usethesource/rascal/blob/5c1c01788afbb0de2a3a4e529e85c03f0edbb3fc/src/org/rascalmpl/uri/URIResolverRegistry.java#L977-L997

We do not pass on the recursive flag to our down stream implementations (the ISourceLocationWatcher has no recursion parameter to the functions). We only ask them to watch the current directory, and then call ourselves recursively.

[jurgenvinju]

You're right! I misread the code there. Mmm.. well that makes it a lot easier to understand what is going on.