Skip to content

Commit

Permalink
Merge #869
Browse files Browse the repository at this point in the history
869: Show Contents of Readme on Crate Pages r=carols10cents

Hey everyone, I started working on README rendering earlier and I thought that I should get some feedback now that it is working correctly on my local instance.

I edited the `/crates/new` endpoint so that when a crate is uploaded to the registry, its README (markdown only) is rendered to HTML and a `readmes/{crate}/{crate}-{version}.html` file is created on S3 when the crate file is uploaded there.
Then, when visiting a crate page, the Ember application makes a request to `{s3}/readmes/{crate}/{crate}-{version}.html`: if the file is found, its content is rendered in place, if not nothing happens.
This way, the renderd README is cached and versioned, following @ashleygwilliams [recommendations](#81 (comment)).

On the `crate/version` route, the README contents is displayed in place of the package description.

The generated HTML is sanitized using [Ammonia](https://crates.io/crates/ammonia). This crate removes all tags and attributes that are not whitelisted, making the resulting markup as safe as possible for the end user. Bonus point: is uses the html5ever crate (created by the Servo project) to parse HTML.

~~~Codeblocks syntax highlighting is done server-side using [syntect](https://crates.io/crates/syntect). This crate uses Sublime syntax files to highlight code samples, which means that we automatically have access to a lot of pre-existing grammars and we can easily add/create missing ones.~~~ Syntax highlighting is now done client-side using PrismJS due to syntect's dependency onigumura conflicting with git2.

Finally, I added a new script in `src/bin` named `render-readmes`. Its goal is to render the README of every uploaded crate version and to upload it on S3. This means that if modify the renderer behavior and we want to retroactively apply it, we're just one command (and a lil' bit of time) away ! (Though I might have left too much `unwrap()`/`expect()` in it)

TODO:

* [x] Security checks in `render::markdown_to_html` (script tag, javascript callbacks, more ?)
* [x] Tests
* [x] Documentation
* [x] ~Edit delete-crate & delete-version scripts to delete README files~ It would seem that the scripts only delete database entries, not stored files
* [x] Script to render published crates README (?)

Fixes #81

Preview:

![screenshot from 2017-07-10 00-22-45](https://user-images.githubusercontent.com/1275463/27998080-f83bad7a-6505-11e7-884e-95ee4f7c2a47.png)
  • Loading branch information
bors-voyager[bot] committed Aug 17, 2017
2 parents 9ce04b1 + e28e48f commit 405b2ad
Show file tree
Hide file tree
Showing 22 changed files with 1,246 additions and 305 deletions.
256 changes: 235 additions & 21 deletions Cargo.lock

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ git2 = "0.6.4"
flate2 = "0.2"
semver = "0.5"
url = "1.2.1"
tar = "0.4.13"

r2d2 = "0.7.0"
openssl = "0.9.9"
Expand All @@ -47,6 +48,8 @@ serde_derive = "1.0.0"
serde = "1.0.0"
clippy = { version = "=0.0.142", optional = true }
chrono = "0.4.0"
pulldown-cmark = { version = "0.0.15", default-features = false }
ammonia = "0.5.0"

conduit = "0.8"
conduit-conditional-get = "0.8"
Expand Down
11 changes: 11 additions & 0 deletions app/components/crate-readme.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import Ember from 'ember';

export default Ember.Component.extend({
rendered: '',
didRender() {
this._super(...arguments);
this.$('pre > code').each(function() {
window.Prism.highlightElement(this);
});
}
});
1 change: 1 addition & 0 deletions app/models/version.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import DS from 'ember-data';
export default DS.Model.extend({
num: DS.attr('string'),
dl_path: DS.attr('string'),
readme_path: DS.attr('string'),
created_at: DS.attr('date'),
updated_at: DS.attr('date'),
downloads: DS.attr('number'),
Expand Down
21 changes: 20 additions & 1 deletion app/routes/crate/version.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,28 @@ export default Route.extend({
`Version '${params.version_num}' of crate '${crate.get('name')}' does not exist`);
}

return version ||
const result = version ||
versions.find(version => version.get('num') === maxVersion) ||
versions.objectAt(0);
if (result.get('readme_path')) {
this.get('ajax').request(result.get('readme_path'))
.then((r) => this.get('ajax').raw(r.url, {
method: 'GET',
dataType: 'html',
headers: {
// We need to force the Accept header, otherwise crates.io won't return
// the readme file when not using S3.
Accept: '*/*',
},
}))
.then((r) => {
crate.set('readme', r.payload);
})
.catch(() => {
crate.set('readme', null);
});
}
return result;
});
},

Expand Down
8 changes: 8 additions & 0 deletions app/styles/crate.scss
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@
.docs {
@include flex(7);
padding-right: 40px;
max-width: 600px;
}
.authorship {
@include flex(3);
Expand Down Expand Up @@ -302,6 +303,13 @@
color: red;
}
}
.crate-readme {
line-height: 1.5;

pre {
overflow-x: scroll;
}
}
.last-update {
color: $main-color-light;
font-size: 90%;
Expand Down
1 change: 1 addition & 0 deletions app/templates/components/crate-readme.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{{rendered}}}
158 changes: 69 additions & 89 deletions app/templates/crate/version.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,28 @@
<div class="quick-links">
<ul>
{{#if crate.homepage}}
<li><a href="{{crate.homepage}}">Homepage</a></li>
<li><a href="{{crate.homepage}}">Homepage</a></li>
{{/if}}
{{#if crate.wiki}}
<li><a href="{{crate.wiki}}">Wiki</a></li>
{{/if}}
{{#if crate.mailing_list}}
<li><a href="{{crate.mailing_list}}">Mailing list</a></li>
{{/if}}
{{#if crate.documentation}}
<li><a href="{{crate.documentation}}">Documentation</a></li>
<li><a href="{{crate.documentation}}">Documentation</a></li>
{{/if}}
{{#if crate.repository}}
<li><a href="{{crate.repository}}">Repository</a></li>
<li><a href="{{crate.repository}}">Repository</a></li>
{{/if}}
{{#if crate.reverse_dependencies}}
<li>
{{#link-to 'crate.reverse_dependencies' (query-params dependency=crate.crate_id)}}
Dependent crates
{{/link-to}}
</li>
{{else}}
<li>No dependent crates</li>
{{/if}}
</ul>
</div>
Expand All @@ -54,12 +69,6 @@
{{else}}
<div class='crate-info'>
<div class='docs'>
{{#if crate.description}}
<div class='about'>
<h3>About This Package</h3>
<p>{{ crate.description }}</p>
</div>
{{/if}}
<div class='install'>
<div class='action'>Cargo.toml</div>
<code id="crate-toml">{{ crate.name }} = "{{ currentVersion.num }}"</code>
Expand All @@ -82,6 +91,11 @@
{{/if}}
</span>
</div>
{{#if crate.readme}}
<div class="crate-readme">
{{crate-readme rendered=crate.readme}}
</div>
{{/if}}
</div>
<div class='authorship'>
<div class='top'>
Expand Down Expand Up @@ -166,88 +180,54 @@
{{/each}}
</ul>
</div>
</div>
</div>
</div>

<div id='crate-links'>
{{#if anyLinks}}
<div class='section'>
<h3>Links</h3>
<ul>
{{#if crate.homepage}}
<li><a href="{{crate.homepage}}">Homepage</a></li>
{{/if}}
{{#if crate.wiki}}
<li><a href="{{crate.wiki}}">Wiki</a></li>
{{/if}}
{{#if crate.mailing_list}}
<li><a href="{{crate.mailing_list}}">Mailing list</a></li>
{{/if}}
{{#if crate.documentation}}
<li><a href="{{crate.documentation}}">Documentation</a></li>
{{/if}}
{{#if crate.repository}}
<li><a href="{{crate.repository}}">Repository</a></li>
{{/if}}
{{#if crate.reverse_dependencies}}
<li>
{{#link-to 'crate.reverse_dependencies' (query-params dependency=crate.crate_id)}}
Dependent crates
{{/link-to}}
</li>
{{else}}
<li>No dependent crates</li>
{{/if}}
</ul>
</div>
{{/if}}
<div class='section' id='crate-versions'>
<h3>Versions</h3>
<ul>
{{#each smallSortedVersions as |version|}}
<li>
{{#link-to 'crate.version' version.num}}
{{ version.num }}
{{/link-to}}
<span class='date'>{{moment-format version.created_at 'll'}}</span>
{{#if version.yanked}}
<span class='yanked'>yanked</span>
{{/if}}
</li>
{{/each}}
</ul>
<span class='small'>
{{#if hasMoreVersions}}
{{#link-to 'crate.versions' crate}}
show all {{ crate.versions.length }} versions
{{/link-to}}
{{/if}}
</span>
</div>

<div class='section' id='crate-dependencies'>
<h3>Dependencies</h3>
<ul>
{{#each currentDependencies as |dep|}}
{{link-to-dep dep=dep}}
{{else}}
<li>None</li>
{{/each}}
</ul>
</div>
<div class='section' id='crate-dependencies'>
<h3>Dependencies</h3>
<ul>
{{#each currentDependencies as |dep|}}
{{link-to-dep dep=dep}}
{{else}}
<li>None</li>
{{/each}}
</ul>
</div>

{{#if currentDevDependencies}}
<div class='section' id='crate-dev-dependencies'>
<h3>Dev-Dependencies</h3>
<ul>
{{#each currentDevDependencies as |dep|}}
{{link-to-dep dep=dep}}
{{/each}}
</ul>
{{#if currentDevDependencies}}
<div class='section' id='crate-dev-dependencies'>
<h3>Dev-Dependencies</h3>
<ul>
{{#each currentDevDependencies as |dep|}}
{{link-to-dep dep=dep}}
{{/each}}
</ul>
</div>
{{/if}}
</div>
</div>
{{/if}}

<div class='section' id='crate-versions'>
<h3>Versions</h3>
<ul>
{{#each smallSortedVersions as |version|}}
<li>
{{#link-to 'crate.version' version.num}}
{{ version.num }}
{{/link-to}}
<span class='date'>{{moment-format version.created_at 'll'}}</span>
{{#if version.yanked}}
<span class='yanked'>yanked</span>
{{/if}}
</li>
{{/each}}
</ul>
<span class='small'>
{{#if hasMoreVersions}}
{{#link-to 'crate.versions' crate}}
show all {{ crate.versions.length }} versions
{{/link-to}}
{{/if}}
</span>
</div>
</div>

<div id='crate-downloads'>
Expand All @@ -256,14 +236,14 @@
<div class='stat'>
<span class='num'>
{{svg-jar "download"}}
{{ format-num downloadsContext.downloads }}
{{format-num downloadsContext.downloads}}
</span>
<span class='desc small'>Downloads all time</span>
</div>
<div class='stat'>
<span class="{{if crate.versions.isPending 'loading'}} num">
{{svg-jar "crate"}}
{{ crate.versions.length }}
{{crate.versions.length}}
</span>
<span class='desc small'>Versions published</span>
</div>
Expand Down
21 changes: 21 additions & 0 deletions ember-cli-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,31 @@
const EmberApp = require('ember-cli/lib/broccoli/ember-app');

module.exports = function(defaults) {
const highlightedLanguages = [
'bash',
'clike',
'glsl',
'go',
'ini',
'javascript',
'json',
'markup',
'protobuf',
'ruby',
'rust',
'scss',
'sql',
'yaml'
];

let app = new EmberApp(defaults, {
babel6: {
plugins: ['transform-object-rest-spread'],
},
'ember-prism': {
theme: 'twilight',
components: highlightedLanguages,
}
});

// Use `app.import` to add additional libraries to the generated
Expand Down
Loading

0 comments on commit 405b2ad

Please sign in to comment.