-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
feat: Added HTML output option #76
feat: Added HTML output option #76
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
a4e4b99
to
86bd5c2
Compare
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.
Looks good, just a few small fixes.
Also there is a place in the readme (around line 94) that this can be documented
I think the HTML output option should also be documented somewhere in the wiki. |
It's a good question. I think it shouldn't be renamed since that could be a breaking change in the case that people are currently using it. It might be better to instead possibly add "html" as a second output option. If you want to do it in this PR, you can, but it could also be done later. Also see this comment for a possible solution - #25 (comment) If you think it's not worth outputting all of the types, it might be fine to just change the "markdown" to a different name that could be for any output type including HTML or JSON since I don't know that many repos are currently using the "markdown" output option (besides this one). |
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.
Looks good!
See above comment, though. I'm not sure what's best and I'm happy to hear your thoughts on it.
I guess we could leave it as it is, add a new output for json when #25 gets resolved and document the |
Sounds good! I suppose we don't even need And I agree that having |
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.
We should probably document the two values that are allowed.
Besides that, I think this is good to merge.
Co-authored-by: Jonah Lawrence <[email protected]>
Thanks! |
Summary
This PR adds an
output_type
option that allows users to take advantage of HTML formatting features.Type of change
How Has This Been Tested?
closes #24