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

Allow directory providers to implement a custom onDidChangeFiles #16080

Merged
merged 1 commit into from Nov 3, 2017

Conversation

Projects
None yet
2 participants
@hansonw
Contributor

hansonw commented Nov 2, 2017

Description of the Change

Atom has supported custom directory providers for quite a long time - the main use case being remote directories, e.g. in Nuclide (https://github.com/facebook/nuclide). The recent addition of file watchers causes several uncaught rejections when a remote directory is added, as it attempts to resolve remote URIs to create a file watcher.

To piggyback on the existing custom directory provider setup, we'll additionally allow custom directories to provide a onDidChangeFiles listener (with the same signature as Project::onDidChangeFiles) to override the default file watcher.

Alternate Designs

Another possibility is to suppress file watching for non-local URIs, but this approach allows for greater extensibility as well as real remote file watching if we wanted it. This also opens the interesting option of having the default Directory class in pathwatcher implement onDidChangeFiles, allowing the file watching code to move out of this repo.

Why Should This Be In Core?

There's no other way for a user-land package to override the file watching APIs.

Benefits

Improved compatibility with custom directory providers (in particular remote directories).

Possible Drawbacks

It's unclear how to better document this functionality. It seems that custom directory providers are generally underdocumented though?

@hansonw hansonw requested a review from smashwilson Nov 2, 2017

@smashwilson

👍 LGTM

@smashwilson smashwilson merged commit 809a2d6 into master Nov 3, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@hansonw hansonw deleted the fb-hw-directory-watcher branch May 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment