Skip to content

Conversation

@Courtcircuits
Copy link
Contributor

What has been done ?

This PR aims to resolve #4772. I made the gleam.toml parsing less strict by allowing the usage of relative links within the documentation.

For example this manifest is now correct :

name = "wisp_basic_auth_courtcircuits"
version = "1.0.298"

description = "Basic HTTP Authentication Scheme for Wisp - just testing gealm export"
[....]
links = [
                { title = "Website", href = "https://wisp.gleam.io" },
                { title = "Website internal", href = "./internal" },
]
[...]

The generated doc looks like this :

gleam docs build
image

As you can see this doc contains a relative link to "./internal" which wasn't possible previously.

And the hex doc looks like this :

gleam docs publish
image

You can find the hexdoc page here.
The link called "Website internal" is not present since it would be strange for hex to contain relative links.

Signed-off-by: Courtcircuits <[email protected]>
Signed-off-by: Courtcircuits <[email protected]>
@Courtcircuits Courtcircuits force-pushed the fix/relative-link-gleam-toml branch from ff255f8 to 0f45d8f Compare October 25, 2025 16:57
Signed-off-by: Courtcircuits <[email protected]>
.links
.iter()
.map(|l| (l.title.as_str(), l.href.clone()))
.filter(|(_, href)| !href.clone().is_internal())
Copy link
Member

Choose a reason for hiding this comment

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

Put the filter before the mapping and remove the clone please 🙏

pub title: String,
#[serde(with = "uri_serde")]
pub href: Uri,
pub href: Href,
Copy link
Member

Choose a reason for hiding this comment

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

The Href type here looks like it'll permit data that's not valid URLs. How about instead we use the URL type from the url crate? It's already an indirect dep, so we can add it and use it without adding a new dep.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I read through url crate documentation and it seems like it needs a "base url" to validate a relative path, which is the goal of this PR.

url documentation says :

Many contexts allow URL references that can be relative to a base URL:

<link rel="stylesheet" href="../main.css">

Since parsed URLs are absolute, giving a base is required for parsing relative URLs:

use url::{Url, ParseError};

assert!(Url::parse("../main.css") == Err(ParseError::RelativeUrlWithoutBase))

Use the join method on an Url to use it as a base URL:

use url::Url;

let this_document = Url::parse("http://servo.github.io/rust-url/url/index.html")?;
let css_url = this_document.join("../main.css")?;
assert_eq!(css_url.as_str(), "http://servo.github.io/rust-url/main.css");

I think the main problem with my implementation is that recursive paths are valid, which should only be the case if the "base url" provides a path (e.g. http://myhexdoc.com/foo/bar). What I could do is to check if the user provided an "Home Page" within the link list, if so it will use that home page link as the base url. If not then I could fall back to a strict generic base url without path like http://foo.com/.

What do you think about that strategy ?

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.

Permit adding relative links in gleam.toml

2 participants