-
Notifications
You must be signed in to change notification settings - Fork 649
Switch from pulldown-cmark
to comrak
#981
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
Conversation
Seems to fix #966 as well |
@anthonynguyen Could you run |
I'm not sure about this... I initially chose @kivikakk as the maintainer of comrak, do you have any thoughts on this ? |
I haven't got any specific thoughts; There was a period of inactivity, but |
Since people write their readmes with GFM tables, checkmarks, etc, I think we should try to have parity with GFM for now. Maybe someday there will actually be one markdown standard.................. lol. Using the same renderer as docs.rs sounds great to me! It's still pretty early for the new rustdoc, so when that becomes production-ready, we can re-evaluate at that point. Right now, though, READMEs aren't included in rustdoc by default. RFC 1990 should make it easier to include README text in docs, so we may have some re-evaluating to do once that's implemented as well. Testing this out now! |
Yeah I think we want tables enabled: #983 |
Should strikethrough be enabled as well @carols10cents? |
Not presuming, but if we want parity with how GitHub would render a README.md, the extensions to enable are all of them, per |
Seems reasonable |
The tagfilter extension is at odds with the Ammonia cleaning that's done, so I'm not sure which one we'd rather have |
What's at odds about it? |
I guess what I mean is there's some overlap in their functionality. |
Mmmm I think I'd prefer the list of allowed tags that Ammonia uses then. |
To chime in slightly, I've spoken to |
@anthonynguyen @carols10cents Just to give some background, |
I just force pushed, sorry about that-- I took out the commit turning the tag filtering off, it sounds like to try and be as similar to github as possible, we should have the tag filtering on even though we're ALSO using ammonia to further sanitize. Tested this out and the issues mentioned look all fixed! 🎉 bors: r+ |
981: Switch from `pulldown-cmark` to `comrak` r=carols10cents Fixes #978  This only enables the task list feature. There are [other GFM features] that aren't enabled. [other GFM features]: https://docs.rs/comrak/0.1.7/comrak/struct.ComrakOptions.html
Build failed |
oops looks like i broke something, fixing |
I'd rather start debugging from the diff of the expected vs actual text, rather than "expected true got false".
bors: r+ |
981: Switch from `pulldown-cmark` to `comrak` r=carols10cents Fixes #978  This only enables the task list feature. There are [other GFM features] that aren't enabled. [other GFM features]: https://docs.rs/comrak/0.1.7/comrak/struct.ComrakOptions.html
Build succeeded |
1010: Readme rendering - only rerender older than a date, only rerender for a crate r=carols10cents Only rerender in a `Box<T>`, only rerender in Firefox :) These changes add some arguments to the render-readmes script to make it easier to restart a failed run only with those readmes that have not been rendered since a particular date, and to rerender one particular crate's readmes. By default, the render-readmes script still rerenders all readmes of all versions. But if the script is killed partway through, it doesn't need to be completely restarted-- you can grab the time that the script printed out that it started, then rerun with `--older-than thattime`, and it'll only try and render the ones it hasn't gotten to yet! Another thing I've wanted is the ability to render one particular crate's readmes, so that I don't have to scroll through all the logs to see why a particular crate failed. So I added the `--crate` argument too. I'd like to get this into the same deploy as #981 since that requires rerendering everything again. @kureuil, could you look over this and see if it looks like I might be breaking anything?
Cool, thanks @carols10cents! |
Thank YOU! ❤️ |
Fixes #978
This only enables the task list feature. There are other GFM features that aren't enabled.