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

cmd/govim: fix darwin file system watcher event check #760

Merged
merged 1 commit into from
Feb 7, 2020

Conversation

leitzler
Copy link
Member

@leitzler leitzler commented Feb 6, 2020

Darwin has a separate file system watcher, and it might emit events
that includes both "Created" and "Changed" flag in the same event.

Since the flag "checked" is checked before "created", the darwin
implementation will skip to report "created" if both files exist.
It looks like it only affects tests at the moment (see comments
in #759).

This fix alters order so that only "created" is reported if both
flags exist.

Fixes #759

Darwin has a separate file system watcher, and it might emit events
that includes both "Created" and "Changed" flag in the same event.

Since the flag "checked" is checked before "created", the darwin
implementation will skip to report "created" if both files exist.
It looks like it only affects tests at the moment (see comments
in #759).

This fix alters order so that only "created" is reported if both
flags exist.

Fixes #759
@leitzler leitzler requested a review from myitcv February 6, 2020 23:06
@myitcv
Copy link
Member

myitcv commented Feb 7, 2020

Totally happy with you both (@leitzler and @findleyr) having looked at this. No review required from me 😄 - not least because I'll happily stay as far away from file watching as possible!

Copy link
Member

@myitcv myitcv left a comment

Choose a reason for hiding this comment

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

LGTM

@leitzler leitzler merged commit 5cfafa0 into master Feb 7, 2020
@leitzler leitzler deleted the cmd_govim_fswatch_darwin_event branch February 7, 2020 07:28
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.

cmd/govim: watcher tests fails on macOS
3 participants