Skip to content

fix mdbook redirection table #1635

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

Merged
merged 2 commits into from
Jan 4, 2024
Merged

fix mdbook redirection table #1635

merged 2 commits into from
Jan 4, 2024

Conversation

uta8a
Copy link
Contributor

@uta8a uta8a commented Jan 3, 2024

Link overloading in Speaker note in URL: https://google.github.io/comprehensive-rust/hello-world/hello-world.html is broken.

The reason why it is broken is below.

Original markdown is here

- Rust uses macros for situations where you want to have a variable number of
arguments (no function [overloading](basic-syntax/functions-interlude.md)).

[overloading](basic-syntax/functions-interlude.md)

Page path is /hello-world/hello-world.html, so overloading link become /hello-world/basic-syntax/functions-interlude.html, which is 404.

I feel it is better to use redirect to exact path, so I edit book.toml and make link redirect to /basic-syntax/functions-interlude.html, which re-redirect to path /control-flow-basics/functions.html

'basic-syntax/functions-interlude.html' = '../control-flow-basics/functions.html'

Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

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

Thank you!

I think we should try to avoid redirect chains, so if you could change book.toml so that it redirects to the final location, that would be great.

Also, please update hello-world.md to point directly to the target page. The redirect is great in case someone bookmarked the page, but let's keep the links in the book pointing to the "real" location.

@uta8a
Copy link
Contributor Author

uta8a commented Jan 3, 2024

Thank you for your review!

I agree that redirects should be avoided. I fixed the docs accordingly.

Also, if I modify hello-world.md's link, isn't it necessary to update book.toml? If so, I will remove the redirect part in book.toml I added.

@uta8a uta8a requested a review from djmitche January 3, 2024 22:36
Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

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

Thank you! I agree that the book.toml change isn't critical here. We have historically added such redirects in case someone had an old URL bookmarked or used in a lnik from elsewhere -- both of which are unlikely here. However, the redirect doesn't hurt anything!

@djmitche djmitche enabled auto-merge (squash) January 4, 2024 14:04
@djmitche djmitche merged commit 4cb12d0 into google:main Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants