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

Replace directory_watcher with listen. #1589

Merged
merged 4 commits into from Oct 24, 2013

Conversation

Projects
None yet
5 participants
@dchest
Contributor

dchest commented Sep 30, 2013

Directory_watcher consumed ~25% CPU on big Jekyll projects (depending on
the number of watched files), since it polled for changes every second.

Listen is easier on CPU, as it uses directory change notifications
provided by OS (currently OS X and Linux), falling back to polling when
they are not available.

Replace directory_watcher with listen.
Directory_watcher consumed ~25% CPU on big Jekyll projects (depending on
the number of watched files), since it polled for changes every second.

Listen is easier on CPU, as it uses directory change notifications
provided by OS (currently OS X and Linux), falling back to polling when
they are not available.
@@ -31,25 +31,24 @@ def self.build(site, options)
#
# Returns nothing.
def self.watch(site, options)
require 'directory_watcher'
require 'listen'
require 'pathname'

This comment has been minimized.

@parkr

parkr Sep 30, 2013

Member

pathname should already be required. If not, please add it to lib/jekyll.rb

source = options['source']
destination = options['destination']
destination = Pathname.new(options['destination'])
.relative_path_from(Pathname.new(source))

This comment has been minimized.

@parkr

parkr Sep 30, 2013

Member

It's not always a relative path from the source. It's often an absolute path. Will it still register as this?

This comment has been minimized.

@dchest

dchest Sep 30, 2013

Contributor

If it's outside of the source, listener won't be trigger by its change.
However, you're correct -- it throws an ArgumentError if destination doesn't share prefix with source. Fixing.

@parkr

This comment has been minimized.

Member

parkr commented Sep 30, 2013

I like it. 👍

@mattr-?

@parkr

This comment has been minimized.

Member

parkr commented Sep 30, 2013

Is this compatible with Windows at all? Or is it just that Windows will need to use polling?

@dchest

This comment has been minimized.

Contributor

dchest commented Sep 30, 2013

@parkr Windows will use polling.

https://github.com/guard/listen/tree/v1.3#on-windows gives instructions on how to enable "wdm", but I'll leave it to Windows programmers to figure out how to integrate it.

@dchest

This comment has been minimized.

Owner

dchest commented on lib/jekyll/commands/build.rb in 58cab6b Sep 30, 2013

I went with catching exception, which is ugly, but works. Probably we can just chop source from destination...

Here's what listener does with paths: https://github.com/guard/listen/blob/v1.3/lib/listen/directory_record.rb#L192

@parkr

This comment has been minimized.

Member

parkr commented Oct 18, 2013

I'm in love with this PR. @mattr- – good for v1.3?

mattr- added a commit that referenced this pull request Oct 24, 2013

Merge pull request #1589 from dchest/better-watch
Replace directory_watcher with listen.

mattr- added a commit that referenced this pull request Oct 24, 2013

@mattr- mattr- merged commit 12ba0a5 into jekyll:master Oct 24, 2013

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Oct 26, 2013

Travis errors since this was merged because

Gem::InstallError: listen requires Ruby version >= 1.9.3.
An error occurred while installing listen (2.1.1), and Bundler cannot continue.

@mattr-

This comment has been minimized.

Member

mattr- commented Oct 27, 2013

Yup. Working on fixing this now.

dchest referenced this pull request Oct 27, 2013

Downgrade Listen to 1.3.x
This is so that we can be Ruby 1.8.x compatible.

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

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