JohnSundell / Marathon

[DEPRECATED] Marathon makes it easy to write, run and manage your Swift scripts πŸƒ
MIT License
1.86k stars 78 forks source link

ScriptManager: Add support for inline dependency resolution #68

Closed JohnSundell closed 7 years ago

JohnSundell commented 7 years ago

This change adds support for specifying dependencies inline, instead of having to use a Marathonfile. A comment can now simply be added next to an import statement, specifying the URL that the dependency can be found at, and Marathon will automatically add that dependency, just like when a Marathonfile is used.

The implementation is a bit naive, as it simply splits the script file based on new lines and iterates through them (although an optimization has been made to stop as soon as a non-import alphanumeric character is found). A more optimized solution would probably be to stream the file and keep reading bytes until all imports have been parsed.

Resolves https://github.com/JohnSundell/Marathon/issues/34

JohnSundell commented 7 years ago

@darthpelo @pixyzehn @kareman @alexaubry Would love your thoughts on this πŸ˜„

kareman commented 7 years ago

This looks great, I have been wanting something like this for a long time.

JohnSundell commented 7 years ago

@kareman Happy to hear that πŸŽ‰ It's gonna make sharing scripts so much smoother, no need for extra files like a Marathonfile.

pixyzehn commented 7 years ago

@JohnSundell Looks awesome!! It might be better to update README about adding support for inline dependency resolutionπŸ˜‰

JohnSundell commented 7 years ago

@pixyzehn awesome suggestion - done! βœ…

alexisakers commented 7 years ago

I love the idea! πŸ‘

Maybe you could improve the performance by using String range/index search methods like range(of:) / rangeOfCharacter(from:) / substring(with:) instead of separating components? (learned this when building HTMLString)

JohnSundell commented 7 years ago

@alexaubry Would that be more performant though, since I'd need to keep searching the entire string until all imports have been parsed? Separating components should be O(N), even if consumes a bit more memory (but shouldn't be a problem here since we're dealing with relatively small data sets on powerful hardware πŸ˜„)

alexisakers commented 7 years ago

@JohnSundell not that much, we're talking a few milliseconds. If that's not a concern it's probably better to keep it simple πŸ˜…

JohnSundell commented 7 years ago

@alexaubry Agreed πŸ‘ πŸ˜„

JohnSundell commented 7 years ago

Tests seem to be quite flaky due to parallelization, gonna take care of that in a separate PR before merging this one πŸ™‚

JohnSundell commented 7 years ago

Dropping the parallelization on Linux for now, gonna investigate it further at a later point πŸ™‚