Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

[WIP] Use atom.watchPath instead of PathWatcher #1232

Closed
wants to merge 9 commits into from
Closed

Conversation

winstliu
Copy link
Contributor

@winstliu winstliu commented Mar 7, 2018

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Replaces pathwatcher with atom.watchPath.

Alternate Designs

None.

Benefits

A better watching library that doesn't lock files everywhere on Windows.

Possible Drawbacks

TODO

Applicable Issues

TODO

@daviwil
Copy link
Contributor

daviwil commented Mar 7, 2018

AppVeyor still crashing... Maybe try the @atom/temp package with this change?

@winstliu
Copy link
Contributor Author

winstliu commented Mar 8, 2018

@daviwil not crashing; just specs failing. I haven't started fixing specs yet.

@daviwil
Copy link
Contributor

daviwil commented Mar 8, 2018

You're right! My bad :)

@smashwilson
Copy link
Contributor

One thing to be aware of - once tree-view is backed by watchPath(), parts of the emulated "atom" filesystem watcher in core won't work any more, because they use terrible hacks to pull these events from tree-view:

https://github.com/atom/atom/blob/204a5aa6950b44ab767e9beee450203b1e7b550c/src/path-watcher.js#L222-L259

I'm not sure what the best solution is there. We could eliminate the "atom" watcher backend, or we could find another way to emulate those events, or just miss them.

@winstliu
Copy link
Contributor Author

winstliu commented Jan 3, 2019

Closing this as I don't feel like I have the necessary experience yet to make this work.

@winstliu winstliu closed this Jan 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants