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

Discussion: Reenable link rewriting for colocated assets #1779

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions components/rendering/src/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use libs::gh_emoji::Replacer as EmojiReplacer;
use libs::once_cell::sync::Lazy;
use libs::pulldown_cmark as cmark;
use libs::tera;
use libs::regex::Regex;

use crate::context::RenderContext;
use crate::table_of_contents::{make_table_of_contents, Heading};
Expand All @@ -19,6 +20,13 @@ const CONTINUE_READING: &str = "<span id=\"continue-reading\"></span>";
const ANCHOR_LINK_TEMPLATE: &str = "anchor-link.html";
static EMOJI_REPLACER: Lazy<EmojiReplacer> = Lazy::new(|| EmojiReplacer::new());

/// Although there exists [a list of registered URI schemes][uri-schemes], a link may use arbitrary,
/// private schemes. This regex checks if the given string starts with something that just looks
/// like a scheme, i.e., a case-insensitive identifier followed by a colon.
///
/// [uri-schemes]: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml
static STARTS_WITH_SCHEMA_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"^[0-9A-Za-z\-]+:").unwrap());

#[derive(Debug)]
pub struct Rendered {
pub body: String,
Expand Down Expand Up @@ -62,6 +70,11 @@ fn find_anchor(anchors: &[String], name: String, level: u16) -> String {
find_anchor(anchors, name, level + 1)
}

/// Colocated asset links refers to the files in the same directory.
fn is_colocated_asset_link(link: &str) -> bool {
!link.starts_with('/') && !link.starts_with('#') && !STARTS_WITH_SCHEMA_RE.is_match(link)

Choose a reason for hiding this comment

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

Does it make sense to consider ../ as well? Those assets also wouldn’t be colocated, but I don’t know if those links could occur while using zola in a reasonable way.

Copy link
Contributor Author

@timakro timakro Mar 6, 2022

Choose a reason for hiding this comment

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

For consistency we want to change links starting with ../ as well. Imagine you want to link to section/asset.png from section/page/index.md, then you would write ../asset.png. That would be rewritten to section/page/../asset.png, which is correct. Without link rewriting the link would be broken.

Choose a reason for hiding this comment

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

Okay, makes sense. What exactly is '/' for then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/ is for absolute links. We should probably prepend base_url, but not the page permalink.

Choose a reason for hiding this comment

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

Okay, thanks for explaining! :)

}

/// Returns whether a link starts with an HTTP(s) scheme.
fn is_external_link(link: &str) -> bool {
link.starts_with("http:") || link.starts_with("https:")
Expand Down Expand Up @@ -92,6 +105,8 @@ fn fix_link(
return Err(format!("Relative link {} not found.", link).into());
}
}
} else if is_colocated_asset_link(link) {
format!("{}{}", context.current_page_permalink, link)
} else {
if is_external_link(link) {
external_links.push(link.to_owned());
Expand Down Expand Up @@ -299,6 +314,14 @@ pub fn markdown_to_html(
code_block = None;
events.push(Event::Html("</code></pre>\n".into()));
}
Event::Start(Tag::Image(link_type, src, title)) => {
if is_colocated_asset_link(&src) {
let link = format!("{}{}", context.current_page_permalink, &*src);
events.push(Event::Start(Tag::Image(link_type, link.into(), title)));
} else {
events.push(Event::Start(Tag::Image(link_type, src, title)));
}
}
Event::Start(Tag::Link(link_type, link, title)) if link.is_empty() => {
error = Some(Error::msg("There is a link that is missing a URL"));
events.push(Event::Start(Tag::Link(link_type, "#".into(), title)));
Expand Down
4 changes: 2 additions & 2 deletions components/rendering/tests/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1018,7 +1018,7 @@ fn can_make_permalinks_with_colocated_assets_for_link() {
InsertAnchor::None,
);
let res = render_content("[an image](image.jpg)", &context).unwrap();
assert_eq!(res.body, "<p><a href=\"image.jpg\">an image</a></p>\n");
assert_eq!(res.body, "<p><a href=\"https://vincent.is/about/image.jpg\">an image</a></p>\n");
}

#[test]
Expand All @@ -1034,7 +1034,7 @@ fn can_make_permalinks_with_colocated_assets_for_image() {
InsertAnchor::None,
);
let res = render_content("![alt text](image.jpg)", &context).unwrap();
assert_eq!(res.body, "<p><img src=\"image.jpg\" alt=\"alt text\" /></p>\n");
assert_eq!(res.body, "<p><img src=\"https://vincent.is/about/image.jpg\" alt=\"alt text\" /></p>\n");
}

#[test]
Expand Down