-
Notifications
You must be signed in to change notification settings - Fork 5
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
DRAFT: HTML support #18
Conversation
Relates to #13 |
@@ -7,6 +7,7 @@ | |||
-- | |||
local utils = require("packages.markdown.utils") | |||
local base = require("packages.base") | |||
local html = require("packages.markdown.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.
In packages/markdown/init.lua
you are loading it via the class, so there wouldn't be any reason to load it too via require()
, is there? (and the commands would be registered via the class if you rename your registerHTMLcommands
to registerCommands
, simply.
@@ -79,10 +79,18 @@ local function SileAstWriter (options) | |||
writer.blockquote = simpleCommandWrapper("markdown:internal:blockquote") | |||
writer.verbatim = simpleCommandWrapper("verbatim") | |||
writer.listitem = simpleCommandWrapper("item") | |||
writer.linebreak = simpleCommandWrapper("cr") |
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.
Why is this removed? We'd still want to support the standard Markdown linebreaks.
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.
Mistake during testing thanks
package._name = "markdown.html" | ||
|
||
local passthroughs = { | ||
"sup", |
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.
SILE does not have these, right? We'd need to map them
- to raise/lower (in the general case)
- or perhaps even, initially to the package-provided textsuperscript/textsubscript (if containing only text).
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 has them.
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 doesn't seem so (at least, there's no "sup"/"sub" in the base and plain class), they only exist it seems in math mode.
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.
Heh, with all due respect, you do so much work it seems you forget some of it!
self.class:loadPackage("textsubsuper") |
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.
Heh, with all due respect, you do so much work it seems you forget some of it!
Heh :) But in that case the commands are textsuperscript
and textsubscript
then (not sup
and sub
, hence my initial question).
In the current implementation, they'd work if the content is text-only. (If it's not, they ought to fail, although we may reconsider our options)
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.
Huh, strange, my document still compiles though. But you're right, the text is not superscripted.
It actually is --
=> I have just added the missing support for it (#19) 😸 |
Via command line as well? |
Ah, no, I don't think it's actually possible by command line. Well that would be a question for SILE (passing main inputter's options via the CLI), but we'll be ready for that case too, then. |
Thanks, I was only testing in command line thus my confusion. 🙇 |
"sub", | ||
"em", | ||
"strong", | ||
{ ["div"] = "hbox" }, |
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.
A div cannot be made an hbox (or it won't be properly line broken) -- same for a span actually.
But more generally, Pandoc has lots of extensions regarding HTML in Markdown, e.g. markdown_in_html_blocks
which allows mixing HTML and Markdown, if I read that correctly.
I have one more concern which comes to mind: I'd ideally like both the pandocast and markdown packages to have the same minimal set of features. This imply, for any test we write, looking at what the Pandoc AST would contain, to at least ensure reasonable parity, whichever route is taken.
I am closing the PR:
But thanks again for the incentive and the ideas. |
I don't know what to do about the options thing given I don't think it's possible to give arguments to raw handlers, but wanted to send you the patch so the work isn't duplicated.