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

Ambiguous relative paths #120

Closed
chriseppstein opened this issue May 29, 2013 · 9 comments
Closed

Ambiguous relative paths #120

chriseppstein opened this issue May 29, 2013 · 9 comments

Comments

@chriseppstein
Copy link

When listening to multiple directories and setting :relative_paths to true, the path that is returned is potentially ambiguous, especially when a file is deleted and we can't even detect its presence in the list of directories. The API should give access to the directory for which the change to the file was detected.

@thibaudgg
Copy link
Member

From the README: Passing the :relative_paths => true option won't work when listening to multiple directories
I know that is confusing, that's why I think that I'll completely removed that option in the 2.0.0 release.

@chriseppstein
Copy link
Author

Except it's really useful to know which "thing" being watched that caused the watch event to trigger. Knowing this aspect is useful for both absolute and relative paths, because it allows us to easily map the event to the reason we know we're watching a particular location. Otherwise, we have to work back from the file name to the paths we're watching. Honestly, I'd like to just have the API where when the block accepts a single argument, we get a list of event objects that describe the action, the watch source, and the file affected. That event object could allow relative and absolute filename access when the watch source is a directory.

@thibaudgg
Copy link
Member

@chriseppstein I'm not sure to understand what you say, could you please give some examples to illustrate. Thanks!

@chriseppstein
Copy link
Author

@thibaudgg So imagine I have a folder of images that I want to compress and another folder of images that I want to create a sprite from. The logic associated with these two folders is completely different. Using absolute paths, I can get the change event and then work backwards. This code would look like:

listener = Listen::Listener.new(image_compression_dir, sprite_dir, :relative_paths => false) do |added, changed, removed|
  # handle image compression
  (added + changed).each do |file|
    next unless file[0...(image_compression_dir.size)] == image_compression_dir
    compress_image(file)
  end
  removed.each do |file|
    next unless file[0...(image_compression_dir.size)] == image_compression_dir
    FileUtils.rm_f(compressed_image_location(file))
  end

  # handle spriting
  if (added + changed + removed).detect {|file| file[0...(sprite_dir.size)] == sprite_dir }
    rebuild_sprite(sprite_dir)
  end
end
listener.start!

I find the way I have to combine the different handlers together to get integration with my project to be an inversion of control. What I'd rather do is this:

class SpriteBuilder
  # ...
  def listen_to(listener)
    listener.listen_to(directory, &method(:rebuild))
  end
  def rebuild
    # if arity == 0, don't pass any args, if 1 pass a list of event objects, if 3, pass (added, changed, removed)
    # ...
  end
  # ...
end
class ImageCompressor
  # ...
  def listen_to(listener)
    listener.on_file_addition(directory, &method(:compress_file))
    listener.on_file_change(directory, &method(:compress_file))
    listener.on_file_removed(directory, &method(:cleanup_file))
  end
  def compress_file(filename)
    # does stuff
  end
  def cleanup_file(filename)
    # removes stuff
  end
  # ...
end

listener = Listen::Listener.new
sprite_builder = SpriteBuilder.new(sprite_dir)
sprite_builder.listen_to(listener)
compressor = ImageCompressor.new(image_compression_dir)
compressor.listen_to(listener)
listener.start!

As you can see, the latter code is going to be more maintainable because the logic of how to respond to change events is kept with the code for the particular source of events. Clearly, I can build this API on top of the current listener API. But I find this approach much more amenable to composing a watcher that serves my several needs for running a watcher.

@thibaudgg
Copy link
Member

So if I understand well the problem is more about the miss of listen_to/on_file_addition/on_file_change/on_file_removed API methods rather than relative/absolute paths, because with in your last example absolute paths would be fine right?

@chriseppstein
Copy link
Author

The current API makes relative paths pretty useless if watching several directories. The api I sketched out above would support both absolute and relative paths because the "watch source" is contextual. A small change to the current API to return events instead of filenames when passed a proc of arity 1 would at least make the relative path feature useful in a multi-source context.

@thibaudgg
Copy link
Member

The current API makes relative paths pretty useless if watching several directories.

Yeah that's why this option is not supported when multiple directories are given :)

But because how some adapter works (OS X) I don't think it would be possible to return the context of the change and for the same reason providing multiple proc by listener wouldn't be possible either.

In your case I think the best/simple thing to do would be to instantiate one listener per context:

sprite_listener = Listen::Listener.new(sprite_dir, &sprite_proc)
compressor_listener = Listen::Listener.new(image_compression_dir, &compressor_proc)
sprite_listener.start!
compressor_listener.start!

Because behind the scene Listen would certainly to have to do the same thing (multiple listeners instantiation).

@chriseppstein
Copy link
Author

Yeah that's why this option is not supported when multiple directories are given :)

It didn't give me an error and it was returning paths relative to something.

sprite_listener.start!
compressor_listener.start!

That second line doesn't run. I assume you meant something like:

sprite_listener.start
compressor_listener.start
begin
  sleep
rescue Interrupt
  exit
end

And it wasn't obvious to me that having many listeners would be efficient, especially if using the polling adapter. Can you think of any downsides that multiple listener threads would incur?

Obviously, absolute paths can be made to work, but if I had this API, it would have saved me time. I thought you might appreciate the feedback.

@thibaudgg
Copy link
Member

It didn't give me an error and it was returning paths relative to something.

Ok, we should definetly fix that. (Adding a warning here: https://github.com/guard/listen/blob/master/lib/listen/listener.rb#L252)

You right with your example, sorry for the start!

Yeah having multiple would be a less efficient, but we couldn't handle it differently than with thread internally anyway.

So at the moment it's either more threads or absolute paths, I don't see other solution for now.

Thanks a lot for your feedback, it's really appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants