Skip to content

Only interpret README files whose filename ends in .md as markdown, render plain text otherwise #995

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

Closed
antoyo opened this issue Aug 21, 2017 · 7 comments · Fixed by #1134
Labels
A-readme C-bug 🐞 Category: unintended, undesired behavior

Comments

@antoyo
Copy link

antoyo commented Aug 21, 2017

Hello.
If the readme is in the asciidoc(tor) format, it is not rendered correctly on crates.io.
Here's an example.
Thanks to fix this.

@Susurrus
Copy link
Contributor

Same with my crate. It might be too difficult to support asciidoc at this stage, but I'd suggest not rendering anything if the file doesn't end in ".md".

@antoyo
Copy link
Author

antoyo commented Aug 24, 2017

If someone is interested, I started a crate to parse asciidoctor.
It is far from finished and I don't have much time right now to work on it, so if anyone is interested, this can be used as a starting point.

@carols10cents carols10cents added C-bug 🐞 Category: unintended, undesired behavior A-readme labels Aug 24, 2017
@carols10cents carols10cents changed the title Asciidoc/Asciidoctor readme not rendered Only interpret README files whose filename ends in .md as markdown, render plain text otherwise Sep 23, 2017
@kivikakk
Copy link
Contributor

According to linguist, we might want to render Markdown for at least .md, .markdown, .mdown, .mdwn, .mkd, .mkdn, and .mkdown. I don't know the story of how each one got there personally, but it seems a fair assumption they were all added due to real-world use (and lack of standardisation).

Will fix this tomorrow.

@kivikakk
Copy link
Contributor

Looks like we need to transmit the README filename while uploading; right now we only upload the content: https://github.com/rust-lang/cargo/blob/ff9ca41fe754f94320ebe69acf4f44842edde3e4/src/cargo/ops/registry.rs#L122-L124.

@Susurrus
Copy link
Contributor

@kivikakk The only supported file endings by Commonmark is .md and .markdown according to the text/markdown MIME type RFC7763.

@kivikakk
Copy link
Contributor

@Susurrus I don't disagree, but RFCs and reality are two very different things. I can find thousands (!) of examples of the alternatives not listed in the RFC by searching around here.

@kivikakk
Copy link
Contributor

It's worth noting RFC7763 refers to Markdown, and not CommonMark. The CommonMark spec doesn't suggest anything about file endings.

(Notably, RFC7764 — while maintaining that ".md" and ".markdown" are the suggested file endings — also refers to ".mkd" files a few times)

bors added a commit to rust-lang/cargo that referenced this issue Oct 18, 2017
transmit: send README filename as well as content

This is required to solve rust-lang/crates.io#995; we currently only send the README content, but not the name of the README itself, so it's not possible to determine how we should render it.

I've confirmed the existing crates.io server silently ignores the new field, so this should be safe to roll out whenever.
bors-voyager bot added a commit that referenced this issue Oct 25, 2017
1134: Record readme filename; use to determine whether to render as Markdown or not r=carols10cents

Fixes #995. Depends on rust-lang/cargo#4633 to have the desired effect, but won't break in the absence of that PR; we'll just assume the readme is called `"README.md"` if the client doesn't report one.

Small CSS fix included to wrap extra long lines.

Comparison of render of https://crates.io/crates/relm's README:

| Before | After |
| :-: | :-: |
| ![screen shot 2017-10-17 at 5 51 50 pm](https://user-images.githubusercontent.com/1915/31650690-42018c7a-b364-11e7-95f8-7e1b8e8d56a0.png) | ![screen shot 2017-10-17 at 5 51 51 pm](https://user-images.githubusercontent.com/1915/31650698-44d16588-b364-11e7-92cf-1c3201c0e60e.png) |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-readme C-bug 🐞 Category: unintended, undesired behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants