search

guard / listen

The Listen gem listens to file modifications and notifies you about the changes.
https://rubygems.org/gems/listen
MIT License
1.92k stars 245 forks source link

Don't return incorrect files when there's a file whose name matches a dir #526

Closed ghiculescu closed 3 years ago

ghiculescu commented 3 years ago

Simpler attempt at https://github.com/guard/listen/pull/524. Reverts https://github.com/guard/listen/pull/460.

Even with the changes in https://github.com/guard/listen/pull/460 reversed, the acceptance tests still pass. Only one unit test fails, and I'm not sure it's asserting the right behavior. With this approach, the new tests in https://github.com/guard/listen/pull/524 also still pass.

cc @bryanlira @ColinDKelley

ColinDKelley commented 3 years ago

Nice, @ghiculescu, I certainly like the simplicity from reverting the code. Let's see how @bryanlira feels about that one spec change.

ColinDKelley commented 3 years ago

BTW this appears a few lines above the first changes:

subtree = if [nil, '', '.'].include? rel_path.to_s

There's no way .to_s ever returns nil, so this could be simplified to:

subtree = if ['', '.'].include?(rel_path.to_s)
ColinDKelley commented 3 years ago

Let's wait another day or so to hear back from @bryanlira. If we don't hear back, I will assume the spec change is good and release this in v3.3.4.

ghiculescu commented 3 years ago

Thanks! In the meantime, do you want me to squash my commits or are you happy with the PR as it stands? (I know some projects don't like the "merge and squash" button in github (though I'm not sure why) so I figure I may as well get everything else ready for merge.)

ColinDKelley commented 3 years ago

I think this branch is good as is. I know @ioquatix prefers the Rebase-and-Merge options so there are no merge commits... Github is saying this branch is ready to go with that strategy.