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: handle another fs event to fix change monitoring on Windows 10 #2575

Merged

Conversation

iamorphen
Copy link
Contributor

@iamorphen iamorphen commented Jul 15, 2024

Introduction

Toward #2551 . We needed to expand my narrower filtering in #2498 to handle some platforms I could not test for. The fix here is similar to that in #2542 .

Code changes

  1. Bin notify::event::ModifyKind::Any as a modification event. Previously we filtered it out.
  2. Add test coverage for this event kind.

ModifyKind::Any is not a catch-all event kind but rather a "no detailed context" one. This seems to be the only granularity we are exposed to on Windows 10.

Testing

I performed manual testing on Windows 10. I modified the config file and files in content, sass, static, templates, and themes and confirmed that site reload is fixed, at least on my runtime. I also tested modifications with editors like vim and simple Notepad. They raise a different set of events, but both contain the modify kind in this change, so this change covers both cases.

It would help to have someone else confirm this change if possible.

@iamorphen iamorphen changed the base branch from master to next July 15, 2024 13:01
@iamorphen iamorphen marked this pull request as ready for review July 15, 2024 13:14
@iamorphen iamorphen force-pushed the fix/orphen/fix-fs-event-filtering-for-windows-10 branch from 4ce8943 to 9ef47e6 Compare July 15, 2024 13:16
@iamorphen
Copy link
Contributor Author

In the event anyone else tests, on Windows, issuing ctrl+c to zola built on next as-is at the time of this writing (c666ee1) results in

Encountered an error: uncategorized error
Enable a logger to see more information.

Usability is not ultimately affected.

@Keats
Copy link
Collaborator

Keats commented Jul 15, 2024

Hm that's a bummer i need to see if we don't get spammed by various events on Linux/Mac. Thanks for the PR!

@Keats
Copy link
Collaborator

Keats commented Jul 15, 2024

Looks fine for me on Mac

@iamorphen
Copy link
Contributor Author

Thanks for your review and testing! I just tested on Linux in the same manner I tested on Windows originally, just making modifications to the config, content, templates, themes, etc., one by one. I don't see any spamming on my end.

@Keats
Copy link
Collaborator

Keats commented Jul 16, 2024

Ok let's wait to see some more Windows people checking in but that looks good for me

@Snowdrama
Copy link

Tested this on Windows 10, worked for me

@Keavon
Copy link

Keavon commented Jul 29, 2024

I compiled this and tested it on my Windows 10 machine and it fixes the problem. I guess I'll be running this version from source until a hotfix release can be made, which I hope can be ASAP! Thanks for solving this pretty serious problem.

@Keats Keats merged commit 1834dc1 into getzola:next Jul 29, 2024
5 checks passed
@iamorphen iamorphen deleted the fix/orphen/fix-fs-event-filtering-for-windows-10 branch July 29, 2024 09:36
@iamorphen
Copy link
Contributor Author

Thanks for your time, folks!

@bwaterford
Copy link

This fix works for me on Windows 7.

(I had to change some of the dependency versions in Cargo.lock to get it to compile running Rust 1.77.2)

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.

5 participants