-
Notifications
You must be signed in to change notification settings - Fork 337
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
base: master
Are you sure you want to change the base?
Nmccann/watch files #138
Conversation
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" |
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.
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 |
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.
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.
The current Linux failure (in |
Agree, can somebody have a look at the Linux build failures? |
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.