Drop write protection + Drop param merging in the pipeline + Write sections back into the dom/mdast/htast #260
Conversation
Codecov Report
@@ Coverage Diff @@
## master #260 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 46 45 -1
Lines 965 999 +34
=====================================
+ Hits 965 999 +34
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
|
Maybe we should handle the mdast changes in a separate PR. |
|
Well, tbh I think those changes are interdependent, so I would like to make sure that the write protection/parameter merging changes are done in such a way to facilitate the mdast refactor… |
|
Although they should be three separate commits… |
|
The 2 first issues might be interdependent but merging the sections in the document is not related (they might change same code area) and must be treated separately (like in #279 ;) ). |
This comment has been minimized.
This comment has been minimized.
|
@koraa I've build a quick and dirty section fix for @davidnuescheler in #301 – the dirty part is where I had to build a workaround to the pipeline merging. I'd suggest we concentrate this PR on the merging and I'll finish the other PR. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
3 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@tripodsan Thanks for all the fixes! |
This comment has been minimized.
This comment has been minimized.
koraa
force-pushed
the
karo/rework
branch
5 times, most recently
from
fe0cb12 to
26d4825
Compare
There was a problem hiding this comment.
@koraa thanks for pulling through with this ginormous effort.
It might be a matter of taste, but I find the frequent-param-reassignments we do now a bit confusing, at least they make it harder to follow the data flow, because the most important returns are implicit rather than through explicit return statements. Having been bitten by the inability to delete stuff from the context in the past, I realize that this might be the price we have to pay for the feature.
On the side of the error handling, I'm not in agreement. It looks like what we have now is a relatively hard catch-all solution, but it's not even making clear why there should be multiple error handlers if they can't take their position in the pipeline into consideration.
Adding non functions is a bug which we should not hide…
BREAKING CHANGE: return value from pipeline functions are no longer merged into the context NOTE: Most of the functional changes live in src/pipeline.js; most other changes are just refactoring the rest of the code base to utilize the changes there. feat(pipe): Drop write protection in pipeline fixes #228 feat(pipe): Get rid of parameter merging fixes #223 This specific change necessitated the bulk of the changes to the code bases; most of the code that was refactored relied on parameter merging before. feat(pipe): Fuller error reporting in pipeline Specifically, all errors are now reported by printing an error message the stack trace with additional information identifying the specific pipeline step. On the http level a 500 status code is transmitted. Co-authored-by: Tobias Bocanegra <tripod@adobe.com>
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
In this pull request I will aim to address three issues:
#228 – By removing write protection from steps in the pipeline
#223 – By getting rid of parameter merging and refactoring all steps accordingly
#143 – By making sections a part of mdast and dom instead of a separate sections property. For this purpose I will extend the mdast with
sectionnodes; in the dom I will probably use section tags and probably something likedomNode._helix.metadatato store the metadata we extracted (although this will be subject for research).I will at first not add a
sectionsproperty to the context for backwards compatibily as we can decide how to cross that bridge after the main part of the work is done.