Raise argument error when passing a file path #563
Conversation
The Listen gem doesn't support listening on changes of individual files, but currently if we pass a file path to `Listen.on`, the listener is successfully initialized, without actually listening on given files. To improve clarity, we notify the user they need to pass a directory by raising an argument error.
| @@ -16,6 +16,12 @@ def initialize(directories, queue, silencer, adapter_options) | |||
| Pathname.new(directory.to_s).realpath | |||
| end | |||
|
|
|||
| @directories.each do |pathname| | |||
| unless pathname.directory? | |||
| fail ArgumentError, "must be a directory: #{pathname}" | |||
There was a problem hiding this comment.
Good point, @janko. I confirmed in the documentation and sample usage that the given path(s) need to be directories.
Any chance there might be cases today that are passing in paths that aren't currently directories, but might be in the future? For example, I could imagine a case like this:
Listen.to("/tmp", "/usr/tmp", "/usr/local/tmp") do ...
Would that have worked before to catch whichever of those folders existed...but would now break?
In the worst case we could wait for the next major version bump to include this check. But we don't have one of those planned so that would be unfortunate.
If the above concern is real, we could possibly reverse the test and only raise in the event that the directory given is actually a plain file?
There was a problem hiding this comment.
@janko Any thoughts on the questions above?
There was a problem hiding this comment.
@ColinDKelley Sorry, this has slipped off my radar, thanks for the input.
It seems that nonexisting directories are not currently supported by Listen, because the realpath call above fails with Errno::ENOENT when I to provide a directory that doesn't exist.
There was a problem hiding this comment.
@janko Thanks for testing that. I checked the realpath docs and confirmed that it will fail if the path doesn't exist. So as you say, we don't have to worry about existing use cases passing in non-existent paths. 👍
| @@ -16,6 +16,12 @@ def initialize(directories, queue, silencer, adapter_options) | |||
| Pathname.new(directory.to_s).realpath | |||
| end | |||
|
|
|||
| @directories.each do |pathname| | |||
| unless pathname.directory? | |||
| fail ArgumentError, "must be a directory: #{pathname}" | |||
There was a problem hiding this comment.
@janko Thanks for testing that. I checked the realpath docs and confirmed that it will fail if the path doesn't exist. So as you say, we don't have to worry about existing use cases passing in non-existent paths. 👍
The Listen gem doesn't appear to support listening on changes of individual files, but currently if we pass a file path to
Listen.on, the listener is successfully initialized, but it won't actually be listening on given files.To improve clarity, we notify the user they need to pass a directory by raising an argument error if a path to file is passed in.