-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
docs: Minor re-grouping of pages #14620
Conversation
r? @weihanglo rustbot has assigned @weihanglo. Use |
src/doc/src/SUMMARY.md
Outdated
@@ -15,33 +15,33 @@ | |||
* [Cargo.toml vs Cargo.lock](guide/cargo-toml-vs-cargo-lock.md) | |||
* [Tests](guide/tests.md) | |||
* [Continuous Integration](guide/continuous-integration.md) | |||
* [Publishing on crates.io](guide/publishing.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redirections are needed. Maybe use this? https://rust-lang.github.io/mdBook/format/configuration/renderers.html#outputhtmlredirect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will generate an HTML page which will automatically redirect to the given location. Note that the source location does not support # anchor redirects.
This is the interesting part of this config. We might need to do this as well?
cargo/src/doc/src/reference/specifying-dependencies.md
Lines 631 to 648 in 2fd7321
<script> | |
(function() { | |
var fragments = { | |
"#overriding-dependencies": "overriding-dependencies.html", | |
"#testing-a-bugfix": "overriding-dependencies.html#testing-a-bugfix", | |
"#working-with-an-unpublished-minor-version": "overriding-dependencies.html#working-with-an-unpublished-minor-version", | |
"#overriding-repository-url": "overriding-dependencies.html#overriding-repository-url", | |
"#prepublishing-a-breaking-change": "overriding-dependencies.html#prepublishing-a-breaking-change", | |
"#overriding-with-local-dependencies": "overriding-dependencies.html#paths-overrides", | |
}; | |
var target = fragments[window.location.hash]; | |
if (target) { | |
var url = window.location.toString(); | |
var base = url.substring(0, url.lastIndexOf('/')); | |
window.location.replace(base + "/" + target); | |
} | |
})(); | |
</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So instead of using the redirect feature, you are proposing that we place a hand-written file in the old location that includes the script to redirect fragments? I'm not fully confident in making sure all of these features interact well with each other, search engines, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mdbook config one is for generating those old pages with a <meta http-equiv="refresh" content="0; URL=/.path/to/new/location.html">
, which is a reasonable way to getting a better search engine optimization. However it doesn't support hash. The hand-written <script>
hack doesn't seem to help this situation, either.
Off the top of my head, the only way to support hash redirections is using JavaScript in the old location of the markdown file, and make the markdown file hidden. It is not good, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have another "bad idea": don't rename file names. Just swap the order in SUMMARY.md.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @ehuss as you may know how well we need to maintain link validity. Like would tools in rust-lang/rust check this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, yea, this might actually be a little bit of a problem. I think there are quite a few links to https://doc.rust-lang.org/cargo/reference/publishing.html#github-permissions in particular.
I actually don't think there is a way to include a redirect file without leaving an entry in SUMMARY which I think is undesirable here. (Scratch that, you can just place an html file in the src directory.)
I might recommend just moving the entries in SUMMARY.md, but don't rename the file. I realize that is not optimal, since the URL is a little confusing that way.
Perhaps in the future we can extend mdbook to support javascript redirects with HTML fragments on removed pages? We can rename the files at that time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see what you mean by leaving an html file in its place. I think that should work. That seems like a reasonable option, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, I'm leaving the path in the old location.
f157076
to
6867124
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with the current state of this PR.
It doesn't "guide" people through a topic but explains in a more top-down fashion what caches exist and is not particularly a common topic people need to know.
cc63648
to
a0c5276
Compare
d8cf767
to
5c3031d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me
when you rebuild those man pages.
@bors r=weihanglo |
☀️ Test successful - checks-actions |
Update cargo 17 commits in 80d82ca22abbee5fb7b51fa1abeb1ae34e99e88a..ad074abe3a18ce8444c06f962ceecfd056acfc73 2024-09-27 17:56:01 +0000 to 2024-10-04 18:18:15 +0000 - test: Remove the last of our custom json assertions (rust-lang/cargo#14576) - docs(ref): Expand on MSRV (rust-lang/cargo#14636) - docs: Minor re-grouping of pages (rust-lang/cargo#14620) - docs(ref): Highleft whats left for msrv-policy (rust-lang/cargo#14638) - Fix `cargo:version_number` - has only one `:` (rust-lang/cargo#14637) - docs: Declare support level for each crate in our Charter / docs (rust-lang/cargo#14600) - chore(deps): update tar to 0.4.42 (rust-lang/cargo#14632) - docs(charter): Declare new Intentional Artifacts as 'small' changes (rust-lang/cargo#14599) - fix: Remove implicit feature removal (rust-lang/cargo#14630) - docs(config): make `--config <PATH>` more prominent (rust-lang/cargo#14631) - chore(deps): update rust crate unicode-width to 0.2.0 (rust-lang/cargo#14624) - chore(deps): update embarkstudios/cargo-deny-action action to v2 (rust-lang/cargo#14628) - docs(ref): Clean up language for `package.rust-version` (rust-lang/cargo#14619) - docs: clarify `target.'cfg(...)'` doesnt respect cfg from build script (rust-lang/cargo#14312) - test: relax compiler panic assertions (rust-lang/cargo#14618) - refactor(compiler): zero-copy deserialization when possible (rust-lang/cargo#14608) - test: add support for features in the sat resolver (rust-lang/cargo#14583)
What does this PR try to resolve?
In figuring out where MSRV documentation should live, I found the location of some items confusing
How should we test and review this PR?
Additional information