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

fix vim editing (Linux) + group related fs events #212

Closed
wants to merge 3 commits into from

Conversation

e2
Copy link
Contributor

@e2 e2 commented Apr 16, 2014

  • Linux/inotify: relies on close_write where attribs are not updated (e.g. ecryptfs)
  • all local adapters: logically groups related fs events into single changes (or skips them completely)
  • avoids notifying multiple events per file
  • fix tests to more consistently use Pathname instead of strings
  • additional tests for _smoosh_changes to test for and handle various editor-related differences in the future
  • use "shift" instead of "pop" to process received events in order (for whatever future reason)
  • treat "mv a x && mv x a" like "touch a"

Cezary Baginski added 2 commits April 16, 2014 15:50
  - Linux/inotify: relies on close_write where
  attribs are not updated (e.g.  ecryptfs)

  - all local adapters: logically groups related
  fs events into single changes (or skip them
  completely)

  - avoids notifying multiple events per file

  - fix tests to more consistently use Pathname
  instead of strings

  - additional tests for _smoosh_changes to test
  for and handle various editor-related
  differences in the future

  - use "shift" instead of "pop" to process
  received events in order (for whatever future
  reason)
end

def _local_fs?
not registry[:adapter].is_a? Adapter::TCP
Copy link
Member

Choose a reason for hiding this comment

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

I try to follow bbatsov Ruby style guideline, please could you change it to: !registry[:adapter].is_a?(Adapter::TCP)

@thibaudgg
Copy link
Member

Looks good, like the logic! 👍

@e2
Copy link
Contributor Author

e2 commented Apr 16, 2014

No - problem. This pull request is for review mostly, so don't hold back on what changes you want, separate commits, edge case or any syntax/conventions/compatibility issues. Thanks!

P.S. I'm mostly worried about directory handling and/or other caveats, since I personally can't think of good use cases with Guard/listen. And other use cases that could have been relying on the previous behavior.

P.S.2. I don't mind being mentioned on related Linux/Vim issues if that helps nail/close those issues.

@e2
Copy link
Contributor Author

e2 commented Apr 16, 2014

I created a special branch that helps debugging issues related to watching/responding to changes (+showing inotify events specifically).

How to use:

gem 'listen', git: 'https://github.com/chronomantic/listen/tree/debugging_events'

If anyone reproduces their problem with this branch of listen, the output should making finding identifying and fixing the cause (listen, adapter, guard or plugin) easy. And closing issues too.

@e2
Copy link
Contributor Author

e2 commented Apr 18, 2014

#215 replaces this PR

@e2 e2 closed this Apr 18, 2014
@e2 e2 deleted the linux_vim_fixes branch April 18, 2014 20:05
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

Successfully merging this pull request may close these issues.

2 participants