SwedbankPay / kramdown-plantuml

PlantUML support for Kramdown, the Markdown parser used in Jekyll
https://rubygems.org/gems/kramdown-plantuml
Apache License 2.0
8 stars 2 forks source link

modify how plantuml injects itself in the kramdown process #106

Closed doudou closed 2 years ago

doudou commented 2 years ago

This commit changes how the injection happens from plain monkey patching to using a module and prepending it into the class hierarchy.

This technique fixes compatibility with middleman. Middleman creates a subclass of Kramdown::Converter::Html, and this was making the method-overriding technique fail, while this one works.

To use in middleman, just require "kramdown-plantuml" in config.rb

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

codecov[bot] commented 2 years ago

Codecov Report

Merging #106 (b5704c4) into main (4a4114b) will increase coverage by 0.00%. The diff coverage is 88.88%.

:exclamation: Current head b5704c4 differs from pull request most recent head 6e17c92. Consider uploading reports for the commit 6e17c92 to get more accurate results

@@           Coverage Diff           @@
##             main     #106   +/-   ##
=======================================
  Coverage   97.19%   97.20%           
=======================================
  Files          15       16    +1     
  Lines         464      465    +1     
=======================================
+ Hits          451      452    +1     
  Misses         13       13           
Impacted Files Coverage Δ
lib/kramdown-plantuml/converter_extension.rb 87.50% <87.50%> (ø)
lib/kramdown_html.rb 100.00% <100.00%> (+8.69%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4a4114b...6e17c92. Read the comment docs.

asbjornu commented 2 years ago

Thank you so much for improving the way we hook into the Kramdown pipeline! This seems to be working just fine, so I'm merging.

(The failing checks are for missing credentials due to this being a PR from a fork. The publishing actions should be disabled for forks in the build.)

mergify[bot] commented 2 years ago

Thank you @doudou for your contribution!