Skip to content

rustdoc: Add support for local resources #107640

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+332 −17
Open

rustdoc: Add support for local resources #107640

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d23042d
Copy local resources into doc generated folder
GuillaumeGomez Feb 3, 2023
62e7207
Turn MarkdownWithToc into a struct with named fields
GuillaumeGomez Feb 3, 2023
3d28a2d
Integrate LocalResources into markdown generation
GuillaumeGomez Feb 3, 2023
618edd1
Add test for local resources
GuillaumeGomez Feb 2, 2023
6713e68
Update rustdoc-ui test output
GuillaumeGomez Feb 3, 2023
15f3fc6
Add rustdoc UI test for not found local resources
GuillaumeGomez Feb 3, 2023
bd298a3
Update librustdoc inner tests
GuillaumeGomez Feb 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions tests/rustdoc/local-resources.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Test ensuring that local resources copy is working as expected.
// Original issue: <https://github.com/rust-lang/rust/issues/32104>

#![crate_name = "foo"]
#![feature(no_core)]
#![no_std]
#![no_core]

// @has local_resources/foo/0.svg
// @has foo/struct.Enum.html
// @has - '//img[@src="../local_resources/foo/0.svg"]' ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please change this so that it attempts to preserve, at least partially, the original filename?

  • Pure numbers are annoying for anyone who wants to download the file (right-click, Save As) or image search engines that use the filename to guess at the subject matter.
  • Even if everything is sorted so that the numbering is technically deterministic, it still means minor changes to the docs can cause images to swap filenames. This will be very confusing when these URLs are shared elsewhere.
  • It would be better if this filename had a file hash in it, and local_resources ought to be named local.files, for consistency with static files and so that the CDN can be configured the same way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with keeping the original filename is that we can potentially end up with multiple files with the same name in the static files folder. I suppose it'll be debated in the RFC.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a hash to the file name. If two files have the same name and the same hash, then that’s not actually a problem.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the key is the absolute file path, which is how we know if a file was already linked to.

/// test!
///
/// ![yep](../../src/librustdoc/html/static/images/rust-logo.svg)
pub struct Enum;

pub mod sub {
// @has foo/sub/struct.Enum.html
// @has - '//img[@src="../../local_resources/foo/0.svg"]' ''
/// test!
///
/// ![yep](../../src/librustdoc/html/static/images/rust-logo.svg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a test case that combines this feature with cross-crate inlining?

pub struct Enum;
}