-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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 Mathjax Support to Rustdoc #16991
Conversation
To help with #15285 this may want to expose a As part of the hoedown upgrade, it looks like some structs will also need to be updated. These are the diffs I saw between the two revs: More struct fields: +typedef void *(*hoedown_realloc_callback)(void *, size_t);
+typedef void (*hoedown_free_callback)(void *);
+
/* hoedown_buffer: character array buffer */
struct hoedown_buffer {
uint8_t *data; /* actual character data */
size_t size; /* size of the string */
size_t asize; /* allocated size (0 = volatile buffer) */
size_t unit; /* reallocation unit size (0 = read-only buffer) */
+
+ hoedown_realloc_callback data_realloc;
+ hoedown_free_callback data_free;
+ hoedown_free_callback buffer_free;
}; One more render callback. This needs to update the count of words int he relevant struct. @@ -119,6 +123,7 @@ struct hoedown_renderer {
int (*strikethrough)(hoedown_buffer *ob, const hoedown_buffer *text, void *opaque);
int (*superscript)(hoedown_buffer *ob, const hoedown_buffer *text, void *opaque);
int (*footnote_ref)(hoedown_buffer *ob, unsigned int num, void *opaque);
+ int (*math)(hoedown_buffer *ob, const hoedown_buffer *text, int displaymode, void *opaque);
/* low level callbacks - NULL copies input directly into the output */ |
Huh. I'm surprised this compiled/ran without updating the relevant structs. Are we invoking UB or something right now? |
It could be that we're getting lucky, or that we're just not instantiating those structs directly, but I think that not faithfully representing |
@alexcrichton I added more padding to correspond to the fields (that we don't seem to care about). Is that what you had in mind? Also I'm all for having rust itself default to using mathjax everywhere, and tossing in a CLI flag for good measure. Gonna take me a while to figure out how to do that, though. I assume all that stuff is in |
That'll be it! Just some munging/passing of flags. |
Uh sure, I can try, but I'm pretty out of my element here. Is that a... pointer to a function pointer? Yuck. Poking at CLI stuff now |
You'll probably want to extend it with something like: type hoedown_realloc_callback = extern fn(*mut c_void, size_t) -> *mut size_t;
type hoedown_free_callback = extern fn(*mut c_void);
struct hoedown_buffer {
// ...
data_realloc: Option<hoedown_realloc_callback>,
data_free: Option<hoedown_free_callback>,
buffer_free: Option<hoedown_free_callback>,
} |
Wait, where did you get the |
Given a C typedef like this: typedef void *(*hoedown_realloc_callback)(void *, size_t); You can visually transform it to something like: typedef hoedown_realloc_callback = void *(*)(void *, size_t); And then a thing about C is to basically ignore the typedef hoedown_realloc_callback = void *(void *, size_t); I suspect it's returning a |
Yeah it wasn't so much that it was returning a I fear that I'm going to accidentally learn how to actually read/write C by the end of this, @alexcrichton. Anyway, I applied your changes. :P Edit: working on CLI stuff |
This is a hoedown limitation? (Also, fwiw, I'm still interested in always enabling math parsing, and emitting an error if |
@huonw As far as I can tell, the whitespace thing is by design, although I had to discover it by trial and error and source reading, myself. Probably to avoid "accidental" math? I have mixed feelings on the design you're proposing, particularly because a general philosophy of markdown is "all input is valid". But more than anything I'm genuinely terrified to try to write a hook into a C library. Would you be able/willing to write the hook described in the original issue? |
@huonw This was just logged: hoedown/hoedown#120 |
I suppose it could actually just be a warning. Anyway, the basic infrastructure might be something like, AFAICT there's not a good way to emit messages about specific parts of the markdown, so I'm happy punting on that (we can also continue to conditionally enable the extension and remove the diff --git a/src/librustdoc/html/layout.rs b/src/librustdoc/html/layout.rs
index 896d070..df84b9b 100644
--- a/src/librustdoc/html/layout.rs
+++ b/src/librustdoc/html/layout.rs
@@ -12,6 +12,7 @@ use std::fmt;
use std::io;
use externalfiles::ExternalHtml;
+use html::markdown;
#[deriving(Clone)]
pub struct Layout {
@@ -34,6 +35,8 @@ pub fn render<T: fmt::Show, S: fmt::Show>(
dst: &mut io::Writer, layout: &Layout, page: &Page, sidebar: &S, t: &T)
-> io::IoResult<()>
{
+ markdown::math_seen.replace(Some(false));
+
write!(dst,
r##"<!DOCTYPE html>
<html lang="en">
@@ -156,6 +159,12 @@ r##"<!DOCTYPE html>
} else {
format!(r#"<script src="{}playpen.js"></script>"#, page.root_path)
},
+ // this must be last so that `math_seen` captures all possible $$'s on this page.
+ mathjax_js = if layout.use_mathjax && markdown::math_seen.get().map_or(false, |x| *x) {
+ "..."
+ } else {
+ ""
+ }
)
}
diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs
index 305c184..f205078 100644
--- a/src/librustdoc/html/markdown.rs
+++ b/src/librustdoc/html/markdown.rs
@@ -101,6 +101,8 @@ struct MyOpaque {
dfltblk: extern "C" fn(*mut hoedown_buffer, *const hoedown_buffer,
*const hoedown_buffer, *mut libc::c_void),
toc_builder: Option<TocBuilder>,
+ math_enabled: bool,
+ math_seen: bool,
}
#[repr(C)]
@@ -151,6 +153,7 @@ local_data_key!(used_header_map: RefCell<HashMap<String, uint>>)
local_data_key!(test_idx: Cell<uint>)
// None == render an example, but there's no crate name
local_data_key!(pub playground_krate: Option<String>)
+local_data_key!(pub math_seen: bool)
pub fn render(w: &mut fmt::Formatter, s: &str, print_toc: bool) -> fmt::Result {
extern fn block(ob: *mut hoedown_buffer, text: *const hoedown_buffer,
@@ -274,16 +277,48 @@ pub fn render(w: &mut fmt::Formatter, s: &str, print_toc: bool) -> fmt::Result {
text.with_c_str(|p| unsafe { hoedown_buffer_puts(ob, p) });
}
+ extern fn math(ob: *mut hoedown_buffer, text: *const hoedown_buffer,
+ display_mode: libc::c_int, opaque: *mut libc::c_void) -> libc::c_int {
+
+ let opaque = opaque as *mut hoedown_html_renderer_state;
+ let opaque = unsafe { &mut *((*opaque).opaque as *mut MyOpaque) };
+
+ opaque.math_seen = true;
+
+ let (open, close) = if !opaque.math_enabled {
+ ("$$", "$$")
+ } else if displaymode == 1 {
+ ("\\[", "\\]")
+ } else {
+ ("\\(", "\\)")
+ };
+
+ open.with_c_str(|open| {
+ close.with_c_str(|close| {
+ unsafe {
+ hoedown_buffer_puts(ob, open, 2);
+ escape_html(ob, (*text).data, (*text).size);
+ hoedown_buffer_puts(ob, close, 2);
+ }
+ })
+ });
+
+ 1
+ }
+
unsafe {
let ob = hoedown_buffer_new(DEF_OUNIT);
let renderer = hoedown_html_renderer_new(0, 0);
let mut opaque = MyOpaque {
dfltblk: (*renderer).blockcode.unwrap(),
- toc_builder: if print_toc {Some(TocBuilder::new())} else {None}
+ toc_builder: if print_toc {Some(TocBuilder::new())} else {None},
+ math_enabled: use_mathjax.get().map_or(false, |x| *x),
+ math_seen: false,
};
(*(*renderer).opaque).opaque = &mut opaque as *mut _ as *mut libc::c_void;
(*renderer).blockcode = Some(block);
(*renderer).header = Some(header);
+ (*renderer).math = Some(math);
let document = hoedown_document_new(renderer, HOEDOWN_EXTENSIONS, 16);
hoedown_document_render(document, ob, s.as_ptr(),
@@ -303,6 +338,10 @@ pub fn render(w: &mut fmt::Formatter, s: &str, print_toc: bool) -> fmt::Result {
});
}
hoedown_buffer_free(ob);
+
+ let old = math_seen.get().map_or(false, |x| *x);
+ math_seen.replace(Some(old || opaque.math_seen));
+
ret
}
} (Not compiled.) |
<3 I didn't expect you to just impl that all right away! so cool swoons |
Forgot about the diff in Layout. |
It is reset before rendering each page in |
Hmm, I merged in your patch and fixed up the minor errors (which since I didn't really tweak all my stuff means there's some redundant stuff), but something somewhere seems to have gone wrong... The math is getting parsed/transformed correctly, but the script isn't getting included in the page. Can't work it out at the moment. Hopefully it'll make sense in the morning. |
@alexcrichton @huonw Could you guys make a hard call on what you want the default behavior to be here? |
It looks like there may actually be a markdown standard soon-ish, and the standard doesn't mention much about math formatting, so I would be tempted to leave it turned off by default because if we switch to something other than hoedown in the future it may break compatibility. Does that sounds ok @huonw? |
Sounds good to me. |
this needs a rebase. |
Oh geez, I totally blanked on this PR. I should probably finish this up soon unless someone with more time wants to take the reigns. |
As discussed in #16300 we might want to stop to consider evaluating https://github.com/Khan/KaTeX as a significantly more lightweight and performant solution than mathjax. Between this, the rebase, and all the work this still needs (command line flags and junk), I'm inclined to just close this PR. I'm a bit swamped now that the school year's back on, and I'd rather focus on the collections stuff if anything. If anyone wants to carry the flag over the finish line, most of the stuff you need is in this branch, but there's still a fair amount of work to do. This commit: Gankra@7affc42 is probably the best one to work off of, as it produces correct results, and isn't cluttered up with the experimental math detection stuff that @huonw suggested but I wasn't able to get working quite right. |
I'm not super clear on the best way to use TLDs and
#![doc]
configs, but this seems to work, and isn't obviously wrong:http://cg.scs.carleton.ca/~abeinges/doc/collections/vec/struct.Vec.html
However, the current design interacts weirdly with re-exports:
http://cg.scs.carleton.ca/~abeinges/doc/std/vec/struct.Vec.html
Detailed design:
Mathjax support will only be enabled on crates that declare
#!doc[(enable_mathjax)]
. When enabled mathjax can be used by wrapping math in double-dollars like so:$$ \sqrt{x} + \log^2n $$
. If the dollars are part of a larger block element, the content will be rendered inline. If they surround an entire block, the math will be rendered in "display mode" as seen in my example pages. Note: the opening$$
's must be lead by whitespace, and the closing$$
's must be followed by whitespace. Thereforeblah blah $$x^2$$.
will not render as math, because there is no whitespace between the last$
and the.
.Closes #16300
Trivializes #15285