Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modify how plantuml injects itself in the kramdown process #106

Merged
merged 1 commit into from
Jul 19, 2022

Conversation

doudou
Copy link
Contributor

@doudou doudou commented Jul 12, 2022

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

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
Copy link

CLAassistant commented Jul 12, 2022

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jul 19, 2022

Codecov Report

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

❗ 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%) ⬆️

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
Copy link
Contributor

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.)

@asbjornu asbjornu merged commit 96700f9 into SwedbankPay:main Jul 19, 2022
@mergify
Copy link
Contributor

mergify bot commented Jul 19, 2022

Thank you @doudou for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants