search

rock-core / vscode-rock

VSCode extension for Rock integration
MIT License
1 stars 1 forks source link

watch for installation manifest file changes #7

Closed g-arjones closed 6 years ago

g-arjones commented 6 years ago

Current implementation requires the user to reload vscode so newly created/added packages are properly recognized by the extension

doudou commented 6 years ago

I've added some support for it in https://github.com/rock-core/vscode-rock/pull/5.

But I agree that we should listen to installation-manifest updates from the FS.

I actually looked a bit into it, and couldn't find a way I liked. I wanted to reload the workspace info if installation-manifest changes. The easiest would have been to add it to the Workspace object, but then I would have had to add a dependency on VScode that much deep down. I couldn't find a nice way to do it differently ...

g-arjones commented 6 years ago

We can't use the file watcher provided by the vscode API because it only watches files that are within vscode's workspace folders (autoproj's root isn't supposed to be added). My idea was to use node's FileSystem module and wrap it in our own FileWatcher class so we can inject it anywhere we want to make it testable.

doudou commented 6 years ago

VS itself uses https://www.npmjs.com/package/nsfw to get filesystem events.

g-arjones commented 6 years ago

So why NSFW? Because it has a consistent and minimal footprint in the Javascript layer, manages recursive watching for you, and is super easy to use.

Since we are only watching files, I don't see any major benefit over node FS.watch and it is an extra dependency. But I'm fine with either.

doudou commented 6 years ago

I don't mind either way ... it's only that whenever I get into a new platform, I have a tendency to assume that the people writing this platform know better than I do and default to whatever choice they already made.

g-arjones commented 6 years ago

Makes perfect sense.