-
Notifications
You must be signed in to change notification settings - Fork 17
feat(file_watcher): Debounce file changes #21
base: master
Are you sure you want to change the base?
Conversation
The original impetus for this is an apparent bug with inotifywait on linux where every time I save a file four file change events are fired. Even if that bug is fixed, though, it's still nice to debounce file change events so that operations which change files a lot (git rebase, for example) only end up running one test run. I played with the debounce timer a lot and at least on my machine 100ms provides a pretty nice balance of responsiveness to file save without ever really seeing double-test-runs.
To avoid a delay before running the tests, use a throttle operation rather than debouncing - this means that within a time window the *first* file change event will trigger a test run, rather than the last.
rschmukler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Nit on a predicate variable name. Your choice on whether to implement it.
| %{watcher_pid: watcher_pid} = state) do | ||
| GenServer.cast(Controller, {:file_changed, file_type(path), path}) | ||
| %{watcher_pid: watcher_pid, throttling: throttling} = state) do | ||
| unless throttling do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: throttling? as the var
|
Realized this actually won't work properly as currently implemented - the way vim (and I'm assuming other editors) save files is they write to a temp file with a random name, then rename that file to the target file. The way this works, only that first event will trigger a file changed event, and since it's not a valid elixir file name nothing will ever run. Will have to revisit this to collect all the changes in a list and trigger a change event on all of them. |
|
@glittershark Could we instead only throttle on |
The original impetus for this is an apparent bug with inotifywait on
linux where every time I save a file four file change events are fired.
Even if that bug is fixed, though, it's still nice to debounce file
change events so that operations which change files a lot (git rebase,
for example) only end up running one test run.
I played with the debounce timer a lot and at least on my machine 100ms
provides a pretty nice balance of responsiveness to file save without
ever really seeing double-test-runs.