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

Plumb Silencer.new and use it #542 spec #544

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions lib/listen/silencer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,21 @@ class Silencer
| ~
)$}x.freeze

# TODO: deprecate these mutators; use attr_reader instead
attr_accessor :only_patterns, :ignore_patterns

def initialize
configure({})
def initialize(options = {})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the backwards compatibility constraints but you should prefer **options.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All previous cases initialized this object with no arguments, so I don't think there's a concern there.

Can you help me understand what cases will work better with **options vs. options = {}?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that **options is effective at collecting the options hash as the last argument when there are many arguments. But with a single argument like this, I'm not seeing a difference between **options and options = {}?

Copy link
Collaborator Author

@ColinDKelley ColinDKelley Aug 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe **options is preferred because it always works regardless of other arguments before it?
I switched over to it here: 32bb3a7

configure(options)
end

# TODO: deprecate this mutator
def configure(options)
@only_patterns = options[:only] ? Array(options[:only]) : nil
@ignore_patterns = _init_ignores(options[:ignore], options[:ignore!])
end

# Note: relative_path is temporarily expected to be a relative Pathname to
# make refactoring easier (ideally, it would take a string)

# TODO: switch type and path places - and verify
def silenced?(relative_path, type)
path = relative_path.to_s
path = relative_path.to_s # in case it is a Pathname

_ignore?(path) || (only_patterns && type == :file && !_only?(path))
end
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/listen/adapter/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def _process_event(dir, event)
end

# Stuff that happens in configure()
allow(Listen::Record).to receive(:new).with(dir1).and_return(record)
allow(Listen::Record).to receive(:new).with(dir1, silencer).and_return(record)

allow(Listen::Change::Config).to receive(:new).with(queue, silencer).
and_return(config)
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/listen/adapter/linux_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
allow(config).to receive(:adapter_options).and_return(adapter_options)
allow(config).to receive(:silencer).and_return(silencer)

allow(Listen::Record).to receive(:new).with(dir1).and_return(record)
allow(Listen::Record).to receive(:new).with(dir1, silencer).and_return(record)
allow(Listen::Change::Config).to receive(:new).with(queue, silencer).
and_return(config)
allow(Listen::Change).to receive(:new).with(config, record).
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/listen/adapter/polling_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
allow(config).to receive(:queue).and_return(queue)
allow(config).to receive(:silencer).and_return(silencer)

allow(Listen::Record).to receive(:new).with(dir1).and_return(record)
allow(Listen::Record).to receive(:new).with(dir1, silencer).and_return(record)

allow(Listen::Change).to receive(:new).with(config, record).
and_return(snapshot)
Expand Down
13 changes: 7 additions & 6 deletions spec/lib/listen/record_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

RSpec.describe Listen::Record do
let(:dir) { instance_double(Pathname, to_s: '/dir') }
let(:silencer) { Listen::Silencer.new }
let(:silencer_options) { { ignore!: [/\A\.ignored/] } }
let(:silencer) { Listen::Silencer.new(silencer_options) }
let(:record) { Listen::Record.new(dir, silencer) }

def dir_entries_for(hash)
Expand Down Expand Up @@ -309,16 +310,15 @@ def record_tree(record)

context 'with subdir containing files' do
before do
real_directory('/dir' => %w[dir1 dir2 .git])
real_directory('/dir/.git' => %w[FETCH_HEAD])
real_directory('/dir' => %w[dir1 dir2 .ignored])
real_directory('/dir/dir1' => %w[foo])
real_directory('/dir/dir1/foo' => %w[bar])
lstat(file('/dir/.git/FETCH_HEAD'))
lstat(file('/dir/.ignored/FETCH_HEAD'))
lstat(file('/dir/dir1/foo/bar'))
real_directory('/dir/dir2' => [])
end

it 'builds record' do
it 'builds record, skipping silenced patterns' do
record.build
expect(record_tree(record)).
to eq(
Expand All @@ -331,7 +331,8 @@ def record_tree(record)

context 'with subdir containing dirs' do
before do
real_directory('/dir' => %w[dir1 dir2])
real_directory('/dir' => %w[dir1 dir2 .ignored])
real_directory('/dir/.ignored' => %w[ignored_file])
real_directory('/dir/dir1' => %w[foo])
real_directory('/dir/dir1/foo' => %w[bar baz])
real_directory('/dir/dir1/foo/bar' => [])
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/listen/silencer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

RSpec.describe Listen::Silencer do
let(:options) { {} }
before { subject.configure(options) }
subject { described_class.new(options) }

describe '#silenced?' do
it { should accept(:file, Pathname('some_dir').join("some_file.rb")) }
Expand Down