Skip to content
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

Fixed bug with the index page's relative links. #2077

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HU90m
Copy link

@HU90m HU90m commented Apr 23, 2023

If there is no index page in the source directory, mdbook uses the first chapter as the index page. When doing so, mdbook doesn't render the page any differently. This causes relative links on the page to break, if the first chapter is within a sub-directory and there is not index.md (or README.md) in the root.

To fix this, a redirect to the first chapter is used instead of a copy of the page.

See bug report for more information: #2060

Copy link
Contributor

@KFearsoff KFearsoff left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for your contribution!

@KFearsoff
Copy link
Contributor

@rustbot label -S-waiting-on-review

@rustbot rustbot removed the S-waiting-on-review Status: waiting on a review label Dec 6, 2023
If there is no index page in the source directory,
mdbook uses the first chapter as the index page.
When doing so, mdbook doesn't render the page any differently.
This causes relative links on the page to break,
if the first chapter is within a sub-directory.

To fix this, a redirect to the first chapter is used
instead of the copy of the page.
Comment on lines -123 to -138
if ctx.is_index {
ctx.data.insert("path".to_owned(), json!("index.md"));
ctx.data.insert("path_to_root".to_owned(), json!(""));
ctx.data.insert("is_index".to_owned(), json!(true));
let rendered_index = ctx.handlebars.render("index", &ctx.data)?;
let rendered_index = self.post_process(
rendered_index,
&ctx.html_config.playground,
&ctx.html_config.code,
ctx.edition,
);
debug!("Creating index.html from {}", ctx_path);
utils::fs::write_file(&ctx.destination, "index.html", rendered_index.as_bytes())?;
}

Ok(())
Copy link

Choose a reason for hiding this comment

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

Could the redirection code be moved here, so we can avoid returning the URL and changing the is_index logic?

Copy link
Author

Choose a reason for hiding this comment

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

Not really (discussed offline)

@nbdd0121
Copy link

nbdd0121 commented Feb 1, 2024

Not sure why the tag is removed...

@rustbot label +S-waiting-on-review

@rustbot rustbot added the S-waiting-on-review Status: waiting on a review label Feb 1, 2024
Comment on lines +545 to +546
let redirect_to = first_file
.to_str()
Copy link
Author

Choose a reason for hiding this comment

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

Changing PR to draft because this implementation is buggy and shouldn't be merged. Simply converting path to string can produce bad URLs.

@HU90m HU90m marked this pull request as draft February 2, 2024 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: waiting on a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants