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

Nmccann/watch files #138

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nmccann
Copy link

@nmccann nmccann commented Sep 19, 2022

This introduces a new (very small) dependency to allow for optional file watching, which is just a lightweight wrapper around Apple's own file watching capabilities. This dependency imports Cocoa, though I've seen on another pull request that limiting to building on Apple machines isn't a deal breaker.

File watching is debounced to prevent excessive regeneration when making frequent changes, as well as to address the fact that file changes are a bit "noisy" - a single change can yield multiple events from the FileWatcher dependency.

The current implementation is very opinionated - only the default Sources, Resources and Content folders are watched for changes, and the debounce duration cannot be changed. All of these could be made configurable, but I think that could be done in a follow up PR, should this one be accepted. (And for folder watching in particular, it would be helpful if the CLI used swift-argument-parser to make it easier to add support for new arguments)

There is one edge case that I haven't been able to figure out - changes to files in the Resources folder trigger more than one regeneration - it seems that the regeneration itself ends up counting as a change which puts it into a loop that it eventually gets out of after a few cycles. The same file in the Sources or Content folder doesn't cause this problem. I have a solution for this (using Apple's CryptoKit to generate SHA256 hashes of the changed files, and compare those before scheduling another regeneration), but I'd like to know whether or not introducing an additional dependency would be a problem.

Added support for watching files for changes, then regenerating the
site. Unfortunately unable to get this working with the existing "ENTER
to exit" functionality, but could do so with "CTRL+C".
static let normalTerminationStatus = 15
static let debounceDuration = 3 * NSEC_PER_SEC
static let runLoopInterval: TimeInterval = 0.1
static let exitMessage = "Press CTRL+C to stop the server and exit"
Copy link
Author

Choose a reason for hiding this comment

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

I was unable to get the folder watching to work while maintaining the Press ENTER to stop the server and exit behaviour - I replaced readLine() with a non-blocking implementation (checking standard input on each iteration of the while loop), but that interfered with the watching functionality for an unknown reason.

FileWatcher dependency is now conditionally included based on the
supported platform. The --watch parameter is essentially ignored on
unsupported parameters.
NSEC_PER_SEC is not available on Linux. Explicitly importing the
Dispatch library (in which it is defined) does not work.
internal struct WebsiteRunner {
static let nanosecondsPerSecond: UInt64 = 1_000_000_000
Copy link
Author

Choose a reason for hiding this comment

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

Workaround for NSEC_PER_SEC not being available on Linux which according to this is a bug. Alternatively, I could have wrapped the NSEC_PER_SEC usage in a #if canImport(FileWatcher) condition, since it's only used alongside FileWatcher.

@nmccann
Copy link
Author

nmccann commented Sep 20, 2022

The current Linux failure (in RSSFeedGenerationTests.swift) appears to be unrelated to my changes - I am unable to locally reproduce that failure (when, for instance, running on a linux Docker container). I see there is a similar (though in a different file) failure on the main branch here - perhaps the affected tests are flakey?

@mwermeester
Copy link

Agree, can somebody have a look at the Linux build failures?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants