Skip to content

fix: relative links in gleam toml #5076

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+72 −15
Open

fix: relative links in gleam toml #5076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4824652
fix: relative links in gleam toml
Courtcircuits Oct 25, 2025
73699f2
fix: lint issues
Courtcircuits Oct 25, 2025
0f45d8f
chore: added entry in changelog
Courtcircuits Oct 25, 2025
4dc3cab
chore: fix format issue
Courtcircuits Oct 25, 2025
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,7 @@
- Fixed a bug where the "pattern match on variable" code action would generate
invalid patterns by repeating a variable name already used in the same pattern.
([Giacomo Cavalieri](https://github.com/giacomocavalieri))

- Made link href validation in gleam.toml files less strict so developers can
use relative links for internal documentation.
([Tristan-Mihai Radulescu](https://github.com/Courtcircuits))
28 changes: 19 additions & 9 deletions compiler-cli/src/publish.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use gleam_core::{
type_,
};
use hexpm::version::{Range, Version};
use http::Uri;
use itertools::Itertools;
use sha2::Digest;
use std::{collections::HashMap, io::Write, path::PathBuf, time::Instant};
Expand Down Expand Up @@ -526,7 +527,7 @@ fn metadata_config<'a>(
source_files: &[Utf8PathBuf],
generated_files: &[(Utf8PathBuf, String)],
) -> Result<String> {
let repo_url = http::Uri::try_from(
let repo_url = Uri::try_from(
config
.repository
.as_ref()
Expand All @@ -547,19 +548,28 @@ fn metadata_config<'a>(
}),
})
.collect();
let links: Vec<(&str, Uri)> = config
.links
.iter()
.map(|l| (l.title.as_str(), l.href.clone()))
.filter(|(_, href)| !href.clone().is_internal())
Copy link
Member

@lpil lpil Oct 30, 2025

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 🙏

.map(|(title, href)| {
(
title,
href.as_uri().expect("Internal link not marked as internal"),
)
})
.chain(repo_url.into_iter().map(|u| ("Repository", u)))
.collect();

let metadata = ReleaseMetadata {
name: &config.name,
version: &config.version,
description: &config.description,
source_files,
generated_files,
licenses: &config.licences,
links: config
.links
.iter()
.map(|l| (l.title.as_str(), l.href.clone()))
.chain(repo_url.into_iter().map(|u| ("Repository", u)))
.collect(),
links,
requirements: requirements?,
build_tools: vec!["gleam"],
}
Expand Down Expand Up @@ -696,7 +706,7 @@ pub struct ReleaseMetadata<'a> {
source_files: &'a [Utf8PathBuf],
generated_files: &'a [(Utf8PathBuf, String)],
licenses: &'a Vec<SpdxLicense>,
links: Vec<(&'a str, http::Uri)>,
links: Vec<(&'a str, Uri)>,
requirements: Vec<ReleaseRequirement<'a>>,
build_tools: Vec<&'a str>,
// What should this be? I can't find it in the API anywhere.
Expand All @@ -705,7 +715,7 @@ pub struct ReleaseMetadata<'a> {

impl ReleaseMetadata<'_> {
pub fn as_erlang(&self) -> String {
fn link(link: &(&str, http::Uri)) -> String {
fn link(link: &(&str, Uri)) -> String {
format!(
"\n {{<<\"{name}\">>, <<\"{url}\">>}}",
name = link.0,
Expand Down
39 changes: 36 additions & 3 deletions compiler-core/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -980,11 +980,44 @@ pub struct DocsPage {
pub source: Utf8PathBuf,
}

#[derive(Deserialize, Serialize, Debug, PartialEq, Eq, Clone)]
#[serde(untagged)]
pub enum Href {
#[serde(with = "uri_serde")]
External(Uri),
Internal(std::path::PathBuf),
}

impl fmt::Display for Href {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Href::External(uri) => write!(f, "{}", uri),
Href::Internal(path) => write!(f, "{}", path.display()),
}
}
}

impl Href {
pub fn is_internal(&self) -> bool {
match self {
Href::Internal(_) => true,
Href::External(_) => false,
}
}

// only URIs are considered as external links
pub fn as_uri(&self) -> Option<Uri> {
match self {
Href::External(uri) => Some(uri.clone()),
Href::Internal(_) => None,
}
}
}

#[derive(Deserialize, Serialize, Debug, PartialEq, Eq, Clone)]
pub struct Link {
pub title: String,
#[serde(with = "uri_serde")]
pub href: Uri,
pub href: Href,
Copy link
Member

@lpil lpil Oct 30, 2025

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

@Courtcircuits Courtcircuits Oct 31, 2025

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 ?

}

// Note we don't use http-serde since we also want to validate the scheme and host is set.
Expand Down Expand Up @@ -1141,7 +1174,7 @@ licences = ["Apache-2.0", "MIT"]
description = "Pretty complex config"
target = "erlang"
repository = { type = "github", user = "example", repo = "my_dep" }
links = [{ title = "Home page", href = "https://example.com" }]
links = [{ title = "Home page", href = "https://example.com" }, { title = "Internal link", href = "./internal" }, { title = "Second internal link", href = "../internal" }, { title = "Third internal link", href = "internal" } ]
internal_modules = ["my_app/internal"]
gleam = ">= 0.30.0"

Expand Down
16 changes: 13 additions & 3 deletions compiler-core/src/snapshots/gleam_core__config__package_config_to_json.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
---
source: compiler-core/src/config.rs
assertion_line: 1171
expression: output
snapshot_kind: text
---
--- GLEAM.TOML

Expand All @@ -12,7 +10,7 @@ licences = ["Apache-2.0", "MIT"]
description = "Pretty complex config"
target = "erlang"
repository = { type = "github", user = "example", repo = "my_dep" }
links = [{ title = "Home page", href = "https://example.com" }]
links = [{ title = "Home page", href = "https://example.com" }, { title = "Internal link", href = "./internal" }, { title = "Second internal link", href = "../internal" }, { title = "Third internal link", href = "internal" } ]
internal_modules = ["my_app/internal"]
gleam = ">= 0.30.0"

Expand Down Expand Up @@ -86,6 +84,18 @@ allow_read = ["./database.sqlite"]
{
"title": "Home page",
"href": "https://example.com/"
},
{
"title": "Internal link",
"href": "./internal"
},
{
"title": "Second internal link",
"href": "../internal"
},
{
"title": "Third internal link",
"href": "internal"
}
],
"erlang": {
Expand Down
Loading