-
Notifications
You must be signed in to change notification settings - Fork 298
Conversation
with node-pre-gyp
Very cool @Akin909 ! 👍 Just had a minor comment about watching for workspace changes, but the code change looks great! |
return this._onChange | ||
} | ||
|
||
get onMove(): IEvent<IFileChangeEvent> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the abstraction a lot - and it seems like it'd be relatively straightforward for us to have a 'mock' implementation for tests, if we need it 👍
|
||
constructor(watch: IFSOptions = {}) { | ||
this._workspace = Workspace.getInstance() | ||
this._activeWorkspace = this._workspace.activeWorkspace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering - would it make sense to hook the onDirectoryChanged
event on Workspace
? In that case, we'd probably want to remove the existing listeners from the old directory, and start listening to the 'new' directory.
Codecov Report
@@ Coverage Diff @@
## master #2116 +/- ##
=======================================
Coverage 37.27% 37.27%
=======================================
Files 288 288
Lines 11608 11608
Branches 1556 1556
=======================================
Hits 4327 4327
Misses 7037 7037
Partials 244 244 Continue to review full report at Codecov.
|
@bryphe due to webpack changes in #2109 🎉 issues I was having getting
chokidar
the FileSystem watching package we discussed in #1506 finally works as expected 💯.This PR is a an initial implementation of the
FileSystemWatcher
, Its basically a thin layer around chokidar event listeners which I in turn use to create event listeners we subscribe to in the explorerResult is that creating new files or deleting new files causes the explorer to update correctly