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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing folder events in file watcher #889

Merged
merged 1 commit into from
Feb 3, 2021
Merged

Fixing folder events in file watcher #889

merged 1 commit into from
Feb 3, 2021

Conversation

tanema
Copy link
Contributor

@tanema tanema commented Jan 25, 2021

fixes #885

why

The file watcher would only originally catch directory events if they were the first event before the event buffer was started. This was because normally the event buffer is used to catch duplicate events or quick events like swap files which move and write at the same time. However there are some editors that touch the directory along with the file that was edited so the file edit event would come first, and the mod of the directory second, causing themekit to miss that event and it getting flushed through the rest of the system.

Fix

The directory event handling has been moved to the event translation step so that it will convert directory events into noops. This is a more central place to do this.

Warn Checklist

  • This changes the interface and requires a Major/Minor version change.
  • I have 馃帺'd these changes by using the commands I changed by hand.
  • I have added a dependancy to the project.
  • I have considered any potential impact on node-themekit

@tanema tanema requested a review from andyw8 January 25, 2021 16:12
if len(events) == 0 {
return false
}

drainTimer := time.NewTimer(drainTimeout)
defer drainTimer.Stop()
for {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The actual issue is on line 143 where we get a second stage of events, the directory event filtering from line 125 was not duplicated there.

events := map[string]Event{}
for _, event := range w.translateEvent(event) {
events[event.Path] = event
for _, e := range w.translateEvent(event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

[optional] perhaps rename e to translatedEvent?

@tanema tanema merged commit 250a58f into master Feb 3, 2021
@tanema tanema deleted the dir_event_catching branch February 3, 2021 16:45
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.

Templates directory is trying to be located in a doubled-up absolute path
2 participants