Skip to content
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

File watcher #106

Merged
merged 8 commits into from
Jun 25, 2016
Merged

File watcher #106

merged 8 commits into from
Jun 25, 2016

Conversation

Boscillator
Copy link
Contributor

@Boscillator Boscillator commented Jun 22, 2016

Added file and directory watching flag. This is as requested in issue #99.
Usage:

coconut --watch examples/example.coco
coconut --watch examples

I did not use the watchdogs library because I wanted to keep dependencies minimal.

@Boscillator Boscillator changed the title Develop File watcher Jun 22, 2016
@evhub
Copy link
Owner

evhub commented Jun 23, 2016

Looks good--just a couple of things that either you can fix now or I will fix when I merge it:

  • Should never have a while True loop without some sort of time.sleep call to prevent it from taking up too much CPU time.
  • Your watch function can't just call plain compile_path(source), it needs to respect all of the other arguments that might have been passed, thus I would make the call to watch replace the call to compile_path in if args.source is not None: such that watch takes all the same arguments, and then passes them along to compile_path when it calls it.

@Boscillator
Copy link
Contributor Author

Ok, I'll work on it.

@candeira
Copy link

Watching files by looping and looking at times is far from good practice, especially when there exist libraries that hook into the host OS and provide you with callbacks when files change:

I use gamin:

@Boscillator
Copy link
Contributor Author

I was trying to keep dependencies to a minimum, but I'll look into it.

@evhub
Copy link
Owner

evhub commented Jun 23, 2016

I think introducing more required dependencies is not a good idea, but if you just put the import statement inside of the watch function, then Coconut remains usable by anyone who doesn't have the dependency and best practices can still be maintained.

@Boscillator
Copy link
Contributor Author

Boscillator commented Jun 24, 2016

Ok, so the problem with that is watchdogs wants me to subclass one of their classes. This would mean I would need to declare the class in the watch function, which would be ugly.
The other option would be to create a watch.py file, containing the watch function and the class and watchdogs imported at the top, and only import that the watch function needs to be called.
Does that seam like a good plan.

@Boscillator
Copy link
Contributor Author

Ok, it now uses watchdogs.

@evhub
Copy link
Owner

evhub commented Jun 24, 2016

Looks good! I'll try to merge it soon.

@evhub evhub merged commit c52a7f2 into evhub:develop Jun 25, 2016
@evhub evhub added the feature label Jul 11, 2016
@evhub evhub added the resolved label Oct 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants