Refactor & overhaul moonc and watchers #380
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Shados
force-pushed
the
overhaul-watchers
branch
from
17d2b0c
to
dfb6fbe
Compare
|
Force-pushed to fix some compatibility issues |
Shados
force-pushed
the
overhaul-watchers
branch
3 times, most recently
from
e3bc439
to
596f6fb
Compare
|
Rebased against master again to drop my |
|
Thanks for the this patch, it's going to take me a bit to go through it, I'll try to get back to you soon. Feel free to bug me on discord too |
- By 'path-related', I mean ones that purely manipulate or work on paths, with no actual filesystem accesses involved - A few new functions are also added
Benefits and changes: - Implemented the rsync-like exclusive/inclusive directory behaviour brought up in issue leafo#342 - Unified CLI file/directory input handling across watch and non-watch code paths - Everything works OK with absolute paths now, although I can't guarantee that it won't violate some user's expectations -- but the behaviour is all consistent - By converting moonc to Moonscript and breaking it up into smaller functions, it should be easier to read, and its component parts could also now be covered by the Busted tests - Also replaces the existing test filesystem stubs/mocks with some more expansive ones, in preparation for increasing test coverage
- Essentially rewrote both SleepWatcher and InotifyWatcher
- Added test suites for both, including an expanded set of filesystem
stubs in order to test them in isolation from the underlying
filesystem
- The filesystem stubs have their own tests, kek
Improvements as a result of these changes:
- Watch mode will remove "orphaned" .lua output files if/when their
corresponding .moon input files are deleted
- But it will not remove .lua files that have no corresponding .moon
file to begin with, in order to avoid e.g. removing vendored Lua
modules
- Both Watcher types will properly handle creation of new subdirectories
- There *shouldn't* be any problematic races with this, although
inotify's lack of recursive directory watching makes it difficult
to know for sure
- More of the code is tested, and more of the code now *could* be tested
(but isn't yet)
- Some bugs were fixed
Downsides:
- Somewhat gnarlier tests
- Moar lines of code
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've tried to split this up across commits to make it a bit easier to follow, but most of the changes are in the last few, as they're fairly heavily intertwined. Intermediate commits do pass the test suites and some brief manual testing.
Main changes/improvements:
--output-tohas been unified acrossmooncand the watchers, previously the behaviour differed with/without-w, and various issues have been resolved with--output-to, including:rsync-like handling of exclusive/inclusive directory path behaviour made in issue moonc adds directory to output paths #342 is added (and can probably be considered enough to close that issue).luaoutput files, if and when their corresponding.moonsource files are deleted.luafiles that had no corresponding.moonfile to begin with; this prevents it from doing things like inadvertantly removing vendored Lua modulesmooncwas migrated to Moonscript and broken up into smaller functions, which should hopefully render it more readable -- and also testable, although not yet actually testedDownsides: