Skip to content

Commit

Permalink
Merge pull request #22 from scpwiki/rel-link-fix
Browse files Browse the repository at this point in the history
Fix anchor and subpath for relative links
  • Loading branch information
Zokhoi authored Sep 11, 2024
2 parents 3377954 + 153f4d7 commit fbb51e0
Show file tree
Hide file tree
Showing 29 changed files with 556 additions and 15 deletions.
1 change: 1 addition & 0 deletions src/parsing/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ fn build_toc_list_element(
let link = Element::Link {
ltype: LinkType::TableOfContents,
link: LinkLocation::Url(Cow::Owned(anchor)),
extra: None,
label: LinkLabel::Text(Cow::Owned(name)),
target: None,
};
Expand Down
1 change: 1 addition & 0 deletions src/parsing/rule/impls/link_anchor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ fn try_consume_fn<'r, 't>(
ok!(Element::Link {
ltype: LinkType::Anchor,
link: LinkLocation::Url(url),
extra: None,
label: LinkLabel::Text(cow!(label)),
target: None,
})
Expand Down
1 change: 1 addition & 0 deletions src/parsing/rule/impls/link_single.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ fn try_consume_link<'r, 't>(
let element = Element::Link {
ltype: LinkType::Direct,
link: LinkLocation::Url(cow!(url)),
extra: LinkLocation::parse_extra(cow!(url)),
label: LinkLabel::Text(cow!(label)),
target,
};
Expand Down
2 changes: 2 additions & 0 deletions src/parsing/rule/impls/link_triple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ fn build_same<'r, 't>(
let element = Element::Link {
ltype,
link,
extra: LinkLocation::parse_extra(cow!(url)),
label: LinkLabel::Url(label),
target,
};
Expand Down Expand Up @@ -180,6 +181,7 @@ fn build_separate<'r, 't>(
let element = Element::Link {
ltype,
link,
extra: LinkLocation::parse_extra(cow!(url)),
label,
target,
};
Expand Down
1 change: 1 addition & 0 deletions src/parsing/rule/impls/url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ fn try_consume_fn<'r, 't>(
let element = Element::Link {
ltype: LinkType::Direct,
link: LinkLocation::Url(url),
extra: None,
label: LinkLabel::Url(None),
target: None,
};
Expand Down
3 changes: 2 additions & 1 deletion src/render/html/element/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub fn render_anchor(
pub fn render_link(
ctx: &mut HtmlContext,
link: &LinkLocation,
extra: Option<&str>,
label: &LinkLabel,
target: Option<AnchorTarget>,
ltype: LinkType,
Expand Down Expand Up @@ -90,7 +91,7 @@ pub fn render_link(
let site = ctx.info().site.as_ref().to_string();
let mut tag = ctx.html().a();
tag.attr(attr!(
"href" => &url,
"href" => &url extra.unwrap_or(""),
"target" => target_value; if target.is_some(),
"class" => "wj-link " css_class interwiki_class,
"data-link-type" => ltype.name(),
Expand Down
3 changes: 2 additions & 1 deletion src/render/html/element/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,10 @@ pub fn render_element(ctx: &mut HtmlContext, element: &Element) {
Element::Link {
ltype,
link,
extra,
label,
target,
} => render_link(ctx, link, label, *target, *ltype),
} => render_link(ctx, link, ref_cow!(extra), label, *target, *ltype),
Element::Image {
source,
link,
Expand Down
14 changes: 10 additions & 4 deletions src/test/prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,20 @@ fn arb_link_element() -> impl Strategy<Value = Element<'static>> {
Just(LinkLabel::Page),
];

(arb_link_type(), arb_link_location(), label, arb_target()).prop_map(
|(ltype, link, label, target)| Element::Link {
(
arb_link_type(),
arb_link_location(),
arb_optional_str(),
label,
arb_target(),
)
.prop_map(|(ltype, link, extra, label, target)| Element::Link {
ltype,
link,
extra,
label,
target,
},
)
})
}

fn arb_image() -> impl Strategy<Value = Element<'static>> {
Expand Down
3 changes: 3 additions & 0 deletions src/tree/element/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ pub enum Element<'t> {
#[serde(rename = "type")]
ltype: LinkType,
link: LinkLocation<'t>,
extra: Option<Cow<'t, str>>,
label: LinkLabel<'t>,
target: Option<AnchorTarget>,
},
Expand Down Expand Up @@ -442,11 +443,13 @@ impl Element<'_> {
Element::Link {
ltype,
link,
extra,
label,
target,
} => Element::Link {
ltype: *ltype,
link: link.to_owned(),
extra: option_string_to_owned(extra),
label: label.to_owned(),
target: *target,
},
Expand Down
82 changes: 75 additions & 7 deletions src/tree/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,56 @@ impl<'a> LinkLocation<'a> {
let mut link_str = link.as_ref();

// Check for direct URLs or anchor links
if is_url(link_str) || link_str.starts_with('#') {
// TODO: parse local links into LinkLocation::Page
// Known bug: single "/" parsed into Url instead of Page
if is_url(link_str) || link_str.starts_with('#') || link_str.starts_with("/") {
return LinkLocation::Url(link);
}

// Check for local links starting with '/'
if link_str.starts_with('/') {
link_str = &link_str[1..];
}
// // Check for local links starting with '/'
// if link_str.starts_with('/') {
// link_str = &link_str[1..];
// }

// Take only the first segment for page
link_str = link_str
.split('#') // get item before the first #
.next()
.expect("Splits always produce at least one item")
.split('/') // get item before the first /
.next()
.expect("Splits always produce at least one item");

match PageRef::parse(link_str) {
Err(_) => LinkLocation::Url(link),
Err(_) => LinkLocation::Url(Cow::Owned(link_str.to_owned())),
Ok(page_ref) => LinkLocation::Page(page_ref.to_owned()),
}
}

pub fn parse_extra(link: Cow<'a, str>) -> Option<Cow<'a, str>> {
let link_str = link.as_ref();

// Check for direct URLs or anchor links
// Does not parse local links for now
if is_url(link_str) || link_str.starts_with('#') || link_str.starts_with('/') {
return None;
}

// Remove first path segment and reconstruct the remaining parts
let mut split_anchor: Vec<&str> = link_str.splitn(2, "#").collect();
let mut split_path: Vec<&str> = split_anchor[0].splitn(2, "/").collect();
split_path[0] = "";
let mut path = split_path.join("/");
split_anchor[0] = &path;
path = split_anchor.join("#");

if path.is_empty() {
None
} else {
Some(Cow::Owned(path))
}
}

pub fn to_owned(&self) -> LinkLocation<'static> {
match self {
LinkLocation::Page(page) => LinkLocation::Page(page.to_owned()),
Expand Down Expand Up @@ -125,7 +160,13 @@ fn test_link_location() {
check!("#anchor" => "#anchor");

check!("page" => None, "page");
check!("/page" => None, "page");
check!("page/edit" => None, "page");
check!("page#toc0" => None, "page");

check!("/page" => "/page");
check!("/page/edit" => "/page/edit");
check!("/page#toc0" => "/page#toc0");

check!("component:theme" => None, "component:theme");
check!(":scp-wiki:scp-1000" => Some("scp-wiki"), "scp-1000");
check!(":scp-wiki:component:theme" => Some("scp-wiki"), "component:theme");
Expand All @@ -139,6 +180,33 @@ fn test_link_location() {
check!("page:multiple:category" => None, "page:multiple:category");
}

#[test]
fn test_link_extra() {
macro_rules! check {
($input:expr => $expected:expr) => {{
let actual = LinkLocation::parse_extra(cow!($input));
let expected = $expected.map(|s| cow!(s));

assert_eq!(
actual, expected,
"Actual link extra segment doesn't match expected",
);
}};
}

check!("" => None);
check!("page" => None);
check!("page/edit" => Some("/edit"));
check!("page#toc0" => Some("#toc0"));
check!("page/edit#toc0" => Some("/edit#toc0"));

check!("/" => None);
check!("/page" => None);
check!("/#/page" => None);
check!("#" => None);
check!("#anchor" => None);
}

#[derive(Serialize, Deserialize, Debug, Hash, Clone, PartialEq, Eq)]
#[serde(rename_all = "kebab-case")]
pub enum LinkLabel<'a> {
Expand Down
13 changes: 11 additions & 2 deletions src/url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,18 @@ pub fn normalize_href(url: &str) -> Cow<str> {
warn!("Attempt to pass in dangerous URL: {url}");
Cow::Borrowed("#invalid-url")
} else {
let mut url = str!(url);
let split_anchor: Vec<&str> = url.splitn(2, "#").collect();
let mut split_url: Vec<&str> = split_anchor[0].split("/").collect();
if !split_url[0].is_empty() || (split_url[0].is_empty() && split_url.len() == 1) {
split_url.insert(0, "");
}
let mut url = str!(split_url[1]);
normalize(&mut url);
url.insert(0, '/');
split_url[1] = &url;
url = split_url.join("/");
if split_anchor.len() == 2 {
url = format!("{}#{}", url, split_anchor[1]);
}
Cow::Owned(url)
}
}
Expand Down
1 change: 1 addition & 0 deletions test/link-single-empty-subpath.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<wj-body class="wj-body"><p><a href="//page/edit" class="wj-link wj-link-internal" data-link-type="direct">Edit</a></p></wj-body>
47 changes: 47 additions & 0 deletions test/link-single-empty-subpath.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"input": "[//page/edit Edit]",
"tree": {
"elements": [
{
"element": "container",
"data": {
"type": "paragraph",
"attributes": {},
"elements": [
{
"element": "link",
"data": {
"type": "direct",
"link": "//page/edit",
"extra": null,
"label": {
"text": "Edit"
},
"target": null
}
}
]
}
},
{
"element": "footnote-block",
"data": {
"title": null,
"hide": false
}
}
],
"html-blocks": [
],
"code-blocks": [
],
"table-of-contents": [
],
"footnotes": [
],
"bibliographies": [
]
},
"errors": [
]
}
1 change: 1 addition & 0 deletions test/link-single-subpath-anchor.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<wj-body class="wj-body"><p><a href="/relative/link#toc0" class="wj-link wj-link-internal" data-link-type="direct">text</a></p></wj-body>
47 changes: 47 additions & 0 deletions test/link-single-subpath-anchor.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"input": "[/relative/link#toc0 text]",
"tree": {
"elements": [
{
"element": "container",
"data": {
"type": "paragraph",
"attributes": {},
"elements": [
{
"element": "link",
"data": {
"type": "direct",
"link": "/relative/link#toc0",
"extra": null,
"label": {
"text": "text"
},
"target": null
}
}
]
}
},
{
"element": "footnote-block",
"data": {
"title": null,
"hide": false
}
}
],
"html-blocks": [
],
"code-blocks": [
],
"table-of-contents": [
],
"footnotes": [
],
"bibliographies": [
]
},
"errors": [
]
}
1 change: 1 addition & 0 deletions test/link-single-subpath.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<wj-body class="wj-body"><p><a href="/relative/link" class="wj-link wj-link-internal" data-link-type="direct">text</a></p></wj-body>
Loading

0 comments on commit fbb51e0

Please sign in to comment.