-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add support for custom markdown parsers #28
Conversation
What if we call it html_converter? This could put us in a path to support other formats beyond markdown in the future. Also, should we pass the filename as argument? I can see matching on the extension being useful? |
Actually, we should make it the same API as the |
👍 This makes sense, since then users would be free to define what files to process without having to change the lib. Eg. maybe they want to use |
README.md
Outdated
end | ||
|
||
defp highlight(html, []), do: html | ||
defp highlight(html, _), do: NimblePublisher.Highlighter.highlight(html) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is tricky. This is private API we would need to expose it but I am not sure if we should. In particular, I am not sure if the regex it uses would match other engines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I did a basic test with md
and the regex worked, but as you said there is no guarantee that it should work. Because this is a "custom" option, if using a engine which it is not compatible with, you can always implement your own highlighting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but how do you feel if you have to implement that module yourself? Maybe we make the Regex an argument to the highlight function and make it public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is looking great. I have suggested some nitpicks on the option name and API. :)
lib/nimble_publisher.ex
Outdated
for highlighter <- highlighters do | ||
Application.ensure_all_started(highlighter) | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure we need this code? We already start all highlighters before, on __extract__
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if someone calls this function without having called use NimblePublisher ...
?
💚 💙 💜 💛 ❤️ |
Use case: I want to use a custom markdown parser (md) instead of Earmark, in order to use my own markdown format with support for
HEEx
inside of markdown.Example usage: