Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

refactor Listeners to work as a library #63

Merged
merged 6 commits into from

4 participants

@niklas

I want to use the Listeners and the abstraction for different platforms in another project (vimmate). This patch allows to specify a directory to listen on. The user can choose whether to receive relative or absolute paths.

Additionally I refactored the listener specs to the extend that every platform-specific listener behaves similar to the others. I could only test the linux and polling version. The latter had to be patched to report moved and deleted files.

@thibaudgg
Owner

Wow awesome refactoring! Thanks.
Actually the Darwin specs doesn't pass on Mac OS X, besides attr_reader :callback is missing in /listeners/darwin, rb-fsevents didn't support deleted & moved files (like rb-fchange on Windows). Right now, Guard didn't have any usages of deleted & moved (support of these on Linux, shouldn't be there either) so I think we should completely remove it.
What do you think about that?

@rymai, @yannlugrin, @netzpirat what your thought about that?

@niklas

actually I though about adding events to the mix, so not only you get the (absolute/relative) path of a changed file, but the type of the change (:modify, :delete, :create etc) also. Maybe you'll need these later?

Would be nice to have a ruby library with (almost) platform independent fs notifications

@rymai
Owner

An agnostic FS events listener gem seems interesting indeed! And you've almost done it already with these commits. Moreover, for the moment deleted & moved events are not detected in Guard so that's one more reason to externalize this "listener" functionality into an external gem, in which all events could be detected and Guard could choose to listen only "modified" events (at the beginning, maybe later it'll be useful to detect other events)...

@thibaudgg
Owner

There is already the fssm gem that is doing that (but without Windows support). If we add deleted & moved event, I'm little bit concerned about performance. That's why I have keep a more "keep it simple" approach in Guard without using fssm. @niklas did you known about fssm?

@niklas

@thibaudgg I did not now fssm. And I looked for it for a while... Will have a look into it and will see if I can merge it with this code here.

@ttilley

I'd rather stab myself in the face than work with a windows box long enough to add support (to FSSM), even though that was one of the original goals... Just helping people out when they see bugs via polling is painful enough. Did you know that the pathname code returns entirely different values in cygwin than one would receive using the one-click installer? So that simply checking if you're running on windows isn't enough to determine whether you're dealing with windows style paths?

@thibaudgg thibaudgg merged commit b12769d into from
@thibaudgg
Owner

Merged & removed moved/deleted files support. Thanks for your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
56 lib/guard/listener.rb
@@ -8,35 +8,75 @@ module Guard
autoload :Polling, 'guard/listeners/polling'
class Listener
- attr_reader :last_event, :sha1_checksums_hash
+ attr_reader :last_event, :sha1_checksums_hash, :directory
- def self.select_and_init
+ def self.select_and_init(*a)
if mac? && Darwin.usable?
- Darwin.new
+ Darwin.new(*a)
elsif linux? && Linux.usable?
- Linux.new
+ Linux.new(*a)
elsif windows? && Windows.usable?
- Windows.new
+ Windows.new(*a)
else
UI.info "Using polling (Please help us to support your system better than that.)"
- Polling.new
+ Polling.new(*a)
end
end
- def initialize
+ def initialize(directory=Dir.pwd, options={})
+ @directory = directory.to_s
@sha1_checksums_hash = {}
+ @relativate_paths = options.fetch(:relativate_paths, true)
update_last_event
end
+ def start
+ watch directory
+ end
+
+ def stop
+ end
+
+ def on_change(&callback)
+ @callback = callback
+ end
+
def update_last_event
@last_event = Time.now
end
def modified_files(dirs, options = {})
files = potentially_modified_files(dirs, options).select { |path| File.file?(path) && file_modified?(path) && file_content_modified?(path) }
- files.map! { |file| file.gsub("#{Dir.pwd}/", '') }
+ relativate_paths files
+ end
+
+ def worker
+ raise NotImplementedError, "should respond to #watch"
+ end
+
+ # register a directory to watch. must be implemented by the subclasses
+ def watch(directory)
+ raise NotImplementedError, "do whatever you want here, given the directory as only argument"
+ end
+
+ def all_files
+ potentially_modified_files [directory + '/'], :all => true
end
+ # scopes all given paths to the current #directory
+ def relativate_paths(paths)
+ return paths unless relativate_paths?
+ paths.map do |path|
+ path.gsub(%r~^#{directory}/~, '')
+ end
+ end
+
+ attr_writer :relativate_paths
+ def relativate_paths?
+ !!@relativate_paths
+ end
+
+
private
def potentially_modified_files(dirs, options = {})
View
25 lib/guard/listeners/darwin.rb
@@ -2,25 +2,23 @@ module Guard
class Darwin < Listener
attr_reader :fsevent
- def initialize
+ def initialize(*)
super
@fsevent = FSEvent.new
end
- def on_change(&callback)
- @fsevent.watch Dir.pwd do |modified_dirs|
- files = modified_files(modified_dirs)
- update_last_event
- callback.call(files)
- end
+ def worker
+ @fsevent
end
def start
- @fsevent.run
+ super
+ fsevent.run
end
def stop
- @fsevent.stop
+ super
+ fsevent.stop
end
def self.usable?
@@ -36,5 +34,14 @@ def self.usable?
false
end
+ private
+ def watch(directory)
+ worker.watch directory do |modified_dirs|
+ files = modified_files(modified_dirs)
+ update_last_event
+ callback.call(files)
+ end
+ end
+
end
end
View
30 lib/guard/listeners/linux.rb
@@ -2,7 +2,7 @@ module Guard
class Linux < Listener
attr_reader :inotify, :files, :latency, :callback
- def initialize
+ def initialize(*)
super
@inotify = INotify::Notifier.new
@@ -12,24 +12,16 @@ def initialize
def start
@stop = false
+ super
watch_change unless watch_change?
end
def stop
+ super
@stop = true
sleep latency
end
- def on_change(&callback)
- @callback = callback
- inotify.watch(Dir.pwd, :recursive, :modify, :create, :delete, :move) do |event|
- unless event.name == "" # Event on root directory
- @files << event.absolute_name
- end
- end
- rescue Interrupt
- end
-
def self.usable?
require 'rb-inotify'
if !defined?(INotify::VERSION) || Gem::Version.new(INotify::VERSION.join('.')) < Gem::Version.new('0.5.1')
@@ -49,6 +41,19 @@ def watch_change?
private
+ def worker
+ @inotify
+ end
+
+ def watch(directory)
+ worker.watch(directory, :recursive, :modify, :create, :delete, :move) do |event|
+ unless event.name == "" # Event on root directory
+ @files << event.absolute_name
+ end
+ end
+ rescue Interrupt
+ end
+
def watch_change
@watch_change = true
until @stop
@@ -61,8 +66,7 @@ def watch_change
unless files.empty?
files.uniq!
- files.map! { |file| file.gsub("#{Dir.pwd}/", '') }
- callback.call(files)
+ callback.call( relativate_paths(files) )
files.clear
end
end
View
30 lib/guard/listeners/polling.rb
@@ -1,22 +1,20 @@
module Guard
class Polling < Listener
- attr_reader :callback, :latency
+ attr_reader :callback, :latency, :existing
- def initialize
+ def initialize(*)
super
@latency = 1.5
end
- def on_change(&callback)
- @callback = callback
- end
-
def start
@stop = false
+ super
watch_change
end
def stop
+ super
@stop = true
end
@@ -25,13 +23,29 @@ def stop
def watch_change
until @stop
start = Time.now.to_f
- files = modified_files([Dir.pwd + '/'], :all => true)
+ current = all_files
+ changed = []
+ # removed files
+ changed += existing - current
+ # added files
+ changed += current - existing
+ # modified
+ changed += existing.select { |path| File.file?(path) && file_modified?(path) && file_content_modified?(path) }
update_last_event
- callback.call(files) unless files.empty?
+
+ changed.uniq!
+
+ callback.call( relativate_paths(changed) ) unless changed.empty?
+ @existing = current
nap_time = latency - (Time.now.to_f - start)
sleep(nap_time) if nap_time > 0
end
end
+ # FIXME: cannot watch muliple directories, but is not needed in guard (yet?)
+ def watch(directory)
+ @existing = all_files
+ end
+
end
end
View
26 lib/guard/listeners/windows.rb
@@ -7,20 +7,13 @@ def initialize
@fchange = FChange::Notifier.new
end
- def on_change(&callback)
- @fchange.watch(Dir.pwd, :all_events, :recursive) do |event|
- paths = [File.expand_path(event.watcher.path) + '/']
- files = modified_files(paths, {:all => true})
- update_last_event
- callback.call(files)
- end
- end
-
def start
+ super
@fchange.run
end
def stop
+ super
@fchange.stop
end
@@ -32,5 +25,20 @@ def self.usable?
false
end
+ private
+
+ def worker
+ @fchange
+ end
+
+ def watch(directory)
+ worker.watch(directory, :all_events, :recursive) do |event|
+ paths = [File.expand_path(event.watcher.path) + '/']
+ files = modified_files(paths, {:all => true})
+ update_last_event
+ callback.call(files)
+ end
+ end
+
end
end
View
68 spec/guard/listener_spec.rb
@@ -27,6 +27,39 @@
Guard::Linux.should_receive(:new)
subject.select_and_init
end
+
+ it "forwards its arguments to the constructor" do
+ subject.stub!(:mac?).and_return(true)
+ Guard::Darwin.stub!(:usable?).and_return(true)
+
+ path, opts = 'path', {:foo => 23}
+ Guard::Darwin.should_receive(:new).with(path, opts).and_return(true)
+ subject.select_and_init path, opts
+ end
+ end
+
+ describe "#all_files" do
+ subject { described_class.new(@fixture_path) }
+
+ it "should return all files" do
+ subject.all_files.should =~ Dir.glob("#{@fixture_path}/**/*")
+ end
+ end
+
+ describe "#relativate_paths" do
+ subject { described_class.new('/tmp') }
+ before :each do
+ @paths = %w( /tmp/a /tmp/a/b /tmp/a.b/c.d )
+ end
+
+ it "should relativate paths to the configured directory" do
+ subject.relativate_paths(@paths).should =~ %w( a a/b a.b/c.d )
+ end
+
+ it "can be disabled" do
+ subject.relativate_paths = false
+ subject.relativate_paths(@paths).should == @paths
+ end
end
describe "#update_last_event" do
@@ -91,4 +124,39 @@
end
end
+ describe "working directory" do
+
+ context "unspecified" do
+ subject { described_class.new }
+ it "defaults to Dir.pwd" do
+ subject.directory.should == Dir.pwd
+ end
+ it "can be not changed" do
+ subject.should_not respond_to(:directory=)
+ end
+ end
+
+ context "specified as first argument to ::new" do
+ before :each do
+ @wd = @fixture_path.join("folder1")
+ end
+ subject { described_class.new @wd }
+ it "can be inspected" do
+ subject.directory.should == @wd.to_s
+ end
+ it "can be not changed" do
+ subject.should_not respond_to(:directory=)
+ end
+
+ it "will be used to watch" do
+ subject.should_receive(:watch).with(@wd.to_s)
+ @listener = subject # indeed.
+ start
+ stop
+ end
+ end
+
+ end
+
+
end
View
56 spec/guard/listeners/darwin_spec.rb
@@ -15,61 +15,9 @@
subject.should be_usable
end
- describe "#on_change" do
- before(:each) do
- @results = []
- @listener = Guard::Darwin.new
- @listener.on_change do |files|
- @results += files
- end
- end
+ it_should_behave_like "a listener that reacts to #on_change"
+ it_should_behave_like "a listener scoped to a specific directory"
- it "catches a new file" do
- file = @fixture_path.join("newfile.rb")
- File.exists?(file).should be_false
- start
- FileUtils.touch file
- stop
- File.delete file
- @results.should == ['spec/fixtures/newfile.rb']
- end
-
- it "catches a single file update" do
- file = @fixture_path.join("folder1/file1.txt")
- File.exists?(file).should be_true
- start
- FileUtils.touch file
- stop
- @results.should == ['spec/fixtures/folder1/file1.txt']
- end
-
- it "catches multiple file updates" do
- file1 = @fixture_path.join("folder1/file1.txt")
- file2 = @fixture_path.join("folder1/folder2/file2.txt")
- File.exists?(file1).should be_true
- File.exists?(file2).should be_true
- start
- FileUtils.touch file1
- FileUtils.touch file2
- stop
- @results.should == ['spec/fixtures/folder1/file1.txt', 'spec/fixtures/folder1/folder2/file2.txt']
- end
- end
- end
-
-private
-
- def start
- sleep 1
- @listener.update_last_event
- Thread.new { @listener.start }
- sleep 1
- end
-
- def stop
- sleep 1
- @listener.stop
- sleep 1
end
end
View
100 spec/guard/listeners/linux_spec.rb
@@ -38,94 +38,28 @@
@listener.should_not be_watch_change
end
- end
-
- describe "#on_change" do
- before(:each) do
- @results = []
- @listener = Guard::Linux.new
- @listener.on_change do |files|
- @results += files
- end
- end
-
- it "catches a new file" do
- file = @fixture_path.join("newfile.rb")
- File.exists?(file).should be_false
- start
- FileUtils.touch file
- stop
- File.delete file
- @results.should == ['spec/fixtures/newfile.rb']
- end
-
- it "catches a single file update" do
- file = @fixture_path.join("folder1/file1.txt")
- File.exists?(file).should be_true
- start
- File.open(file, 'w') {|f| f.write('') }
- stop
- @results.should == ['spec/fixtures/folder1/file1.txt']
- end
-
- it "catches multiple file updates" do
- file1 = @fixture_path.join("folder1/file1.txt")
- file2 = @fixture_path.join("folder1/folder2/file2.txt")
- File.exists?(file1).should be_true
- File.exists?(file2).should be_true
- start
- File.open(file1, 'w') {|f| f.write('') }
- File.open(file2, 'w') {|f| f.write('') }
- stop
- @results.should == ['spec/fixtures/folder1/file1.txt', 'spec/fixtures/folder1/folder2/file2.txt']
+ it "should use Inotify as worker" do
+ @listener.__send__(:worker).should be_a(INotify::Notifier)
end
- it "catches a deleted file" do
- file = @fixture_path.join("folder1/file1.txt")
- File.exists?(file).should be_true
- start
- File.delete file
- stop
- FileUtils.touch file
- @results.should == ['spec/fixtures/folder1/file1.txt']
- end
-
- it "catches a moved file" do
- file1 = @fixture_path.join("folder1/file1.txt")
- file2 = @fixture_path.join("folder1/movedfile1.txt")
- File.exists?(file1).should be_true
- File.exists?(file2).should be_false
- start
- FileUtils.mv file1, file2
- stop
- FileUtils.mv file2, file1
- @results.should == ['spec/fixtures/folder1/file1.txt', 'spec/fixtures/folder1/movedfile1.txt']
- end
-
- it "doesn't process a change when it is stopped" do
- file = @fixture_path.join("folder1/file1.txt")
- File.exists?(file).should be_true
- start
- @listener.inotify.should_not_receive(:process)
- stop
- File.open(file, 'w') {|f| f.write('') }
- end
end
- end
-private
-
- def start
- sleep 1
- @listener.update_last_event
- Thread.new { @listener.start }
- sleep 1
- end
+ it_should_behave_like "a listener that reacts to #on_change"
+ it_should_behave_like "a listener scoped to a specific directory"
+
+ # Fun fact: File.touch seems not to be enough on Linux to trigger a modify event
+
+ it "doesn't process a change when it is stopped" do
+ @listener = described_class.new
+ record_results
+ file = @fixture_path.join("folder1/file1.txt")
+ File.exists?(file).should be_true
+ start
+ @listener.inotify.should_not_receive(:process)
+ stop
+ File.open(file, 'w') {|f| f.write('') }
+ end
- def stop
- sleep 1
- @listener.stop
- sleep 1
end
end
View
57 spec/guard/listeners/polling_spec.rb
@@ -3,60 +3,9 @@
describe Guard::Polling do
- before(:each) do
- @results = []
- @listener = Guard::Polling.new
- @listener.on_change do |files|
- @results += files
- end
- end
+ subject { Guard::Polling }
- describe "#on_change" do
- it "catches a new file" do
- file = @fixture_path.join("newfile.rb")
- File.exists?(file).should be_false
- start
- FileUtils.touch file
- stop
- File.delete file
- @results.should == ['spec/fixtures/newfile.rb']
- end
-
- it "catches a single file update" do
- file = @fixture_path.join("folder1/file1.txt")
- File.exists?(file).should be_true
- start
- FileUtils.touch file
- stop
- @results.should == ['spec/fixtures/folder1/file1.txt']
- end
-
- it "catches multiple file updates" do
- file1 = @fixture_path.join("folder1/file1.txt")
- file2 = @fixture_path.join("folder1/folder2/file2.txt")
- File.exists?(file1).should be_true
- File.exists?(file2).should be_true
- start
- FileUtils.touch file1
- FileUtils.touch file2
- stop
- @results.should =~ ['spec/fixtures/folder1/file1.txt', 'spec/fixtures/folder1/folder2/file2.txt']
- end
- end
-
-private
-
- def start
- sleep 1
- @listener.update_last_event
- Thread.new { @listener.start }
- sleep 1
- end
-
- def stop
- sleep 1
- @listener.stop
- sleep 1
- end
+ it_should_behave_like "a listener that reacts to #on_change"
+ it_should_behave_like "a listener scoped to a specific directory"
end
View
65 spec/guard/listeners/windows_spec.rb
@@ -21,70 +21,9 @@
subject.should be_usable
end
- describe "#on_change" do
- before(:each) do
- @results = []
- @listener = Guard::Windows.new
- @listener.on_change do |files|
- @results += files
- end
- end
+ it_should_behave_like "a listener that reacts to #on_change"
+ it_should_behave_like "a listener scoped to a specific directory"
- it "catches a new file" do
- file = @fixture_path.join("newfile.rb")
- if File.exists?(file)
- begin
- File.delete file
- rescue
- end
- end
- File.exists?(file).should be_false
- start
- FileUtils.touch file
- stop
- begin
- File.delete file
- rescue
- end
- @results.should == ['spec/fixtures/newfile.rb']
- end
-
- it "catches a single file update" do
- file = @fixture_path.join("folder1/file1.txt")
- File.exists?(file).should be_true
- start
- FileUtils.touch file
- stop
- @results.should == ['spec/fixtures/folder1/file1.txt']
- end
-
- it "catches multiple file updates" do
- file1 = @fixture_path.join("folder1/file1.txt")
- file2 = @fixture_path.join("folder1/folder2/file2.txt")
- File.exists?(file1).should be_true
- File.exists?(file2).should be_true
- start
- FileUtils.touch file1
- FileUtils.touch file2
- stop
- @results.should == ['spec/fixtures/folder1/file1.txt', 'spec/fixtures/folder1/folder2/file2.txt']
- end
- end
- end
-
-private
-
- def start
- sleep 1
- @listener.update_last_event
- Thread.new { @listener.start }
- sleep 1
- end
-
- def stop
- sleep 1
- @listener.stop
- sleep 1
end
end
View
116 spec/support/listener_helper.rb
@@ -0,0 +1,116 @@
+private
+
+ def start
+ sleep 1
+ @listener.update_last_event
+ Thread.new { @listener.start }
+ sleep 1
+ end
+
+ def record_results
+ @results = []
+ @listener.on_change do |files|
+ @results += files
+ end
+ end
+
+ def stop
+ sleep 1
+ @listener.stop
+ sleep 1
+ end
+
+ def results
+ @results.flatten
+ end
+
+shared_examples_for 'a listener that reacts to #on_change' do
+ before(:each) do
+ @listener = described_class.new
+ record_results
+ end
+
+ it "catches a new file" do
+ file = @fixture_path.join("newfile.rb")
+ if File.exists?(file)
+ begin
+ File.delete file
+ rescue
+ end
+ end
+ File.exists?(file).should be_false
+ start
+ FileUtils.touch file
+ stop
+ begin
+ File.delete file
+ rescue
+ end
+ results.should =~ ['spec/fixtures/newfile.rb']
+ end
+
+ it "catches a single file update" do
+ file = @fixture_path.join("folder1/file1.txt")
+ File.exists?(file).should be_true
+ start
+ File.open(file, 'w') {|f| f.write('') }
+ stop
+ results.should =~ ['spec/fixtures/folder1/file1.txt']
+ end
+
+ it "catches multiple file updates" do
+ file1 = @fixture_path.join("folder1/file1.txt")
+ file2 = @fixture_path.join("folder1/folder2/file2.txt")
+ File.exists?(file1).should be_true
+ File.exists?(file2).should be_true
+ start
+ File.open(file1, 'w') {|f| f.write('') }
+ File.open(file2, 'w') {|f| f.write('') }
+ stop
+ results.should =~ ['spec/fixtures/folder1/file1.txt', 'spec/fixtures/folder1/folder2/file2.txt']
+ end
+
+ it "catches a deleted file" do
+ file = @fixture_path.join("folder1/file1.txt")
+ File.exists?(file).should be_true
+ start
+ File.delete file
+ stop
+ FileUtils.touch file
+ results.should =~ ['spec/fixtures/folder1/file1.txt']
+ end
+
+ it "catches a moved file" do
+ file1 = @fixture_path.join("folder1/file1.txt")
+ file2 = @fixture_path.join("folder1/movedfile1.txt")
+ File.exists?(file1).should be_true
+ File.exists?(file2).should be_false
+ start
+ FileUtils.mv file1, file2
+ stop
+ FileUtils.mv file2, file1
+ results.should =~ ['spec/fixtures/folder1/file1.txt', 'spec/fixtures/folder1/movedfile1.txt']
+ end
+
+end
+
+shared_examples_for "a listener scoped to a specific directory" do
+ before :each do
+ @wd = @fixture_path.join("folder1")
+ @listener = described_class.new @wd
+ end
+ it "should base paths within this directory" do
+ record_results
+ new_file = @wd.join("folder2/newfile.rb")
+ modified = @wd.join("file1.txt")
+ File.exists?(modified).should be_true
+ File.exists?(new_file).should be_false
+ start
+ FileUtils.touch new_file
+ File.open(modified, 'w') {|f| f.write('') }
+ stop
+ File.delete new_file
+ results.should =~ ["folder2/newfile.rb", 'file1.txt']
+ end
+end
+
Something went wrong with that request. Please try again.