Breaking: support for Hercule transclusion w/ backwards compatibility (WIP) #241
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
|
Regarding the breaking sync API change proposal, I noticed looking at the Travis config, Aglio supports node 0.10. Hercule's method for offering a sync API relies on |
jamesramsay
changed the title
|
👍 For me I am all up for using Hercule transclusions in Aglio. |
jamesramsay
force-pushed
the
hercule
branch
3 times, most recently
from
859dc67
to
30548c3
Compare
|
Thanks @zdne 🎉 Any thoughts from @danielgtaylor ? I've covered everything these changes touch, but the uncovered LOCs remains equal, while covered LOCs decreases, thus the small decrease in coverage. |
|
@jamesramsay I think this is great. I'd like to hold off on merging just for a bit, get an updated stable release out and then move forward with breaking changes to the API. 👍 |
|
this looks very interesting. Cheeky bump! |
Internally Hercule tokenises files into content and link tokens. Only the link portion (excluding the identifying surrounding syntax) is parsed. In the simple case this is simply a file path.In this regard, Aglio syntax permits a strict subset of Hercule's with differing surrounding markers. Hercule also has built-in circular dependency detection, to prevent the it hanging when provided with bad input. - Remove: INCLUDE regular expression. - New: add tokenising regular expression, link match function to extend Hercule's tokeniser with support for existing Aglio syntax. - Update: compileFile, render to use hercule.transcludeString. - Beaking: collectPathsSync removed in favour of collectPaths (async). Hercule is written using streams, which are asynchronous. Without extension of Hercule's sync API (childProcess.spawnSync) which is significantly less performant, collectPathsSync becomes collectPaths (async). - Docs: updated collectPaths to reflect async.
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 noticed some discussion in the API Blueprint Slack chat room about supporting Hercule syntax in Aglio, and can see how it's unfortunate for new comers to Apiary and API Blueprint to make the choice between Aglio's handy rendering capabilities and Hercule's syntax which play's nicer with Markdown.
Rather than leaving you to work out how to replicate the Hercule syntax or a subset thereof, or add Hercule as a dependency, I thought I'd take look.
I've made a few small changes to Hercule to make using it in this application simpler:
collectPathsSync): New: ability to pass in pathList variable (closes #196) jamesramsay/hercule#198These changes were quite small, and made it quite easy to drop Hercule into Aglio. I've published these changes to NPM as
3.0.0-alpha.1(w/nextdistribution tag) and a breaking change to the async API to use typical error-first, single-parameter callback style.However, because Hercule is entirely built on stream and the API is dominantly asynchronous, I've made
collectPathsasynchronous in the PR. Adding the pathList to Hercule's sync API is a little bit more complicated.I'd be interested on your thoughts on this contribution. Thanks for you consideration!
Items that should likely be addressed before merging: