Skip to content

Conversation

@jonludlam
Copy link
Member

Page links were not being remapped because the remap prefix check only matched against the directory path. For modules, the module name is part of the directory (e.g., prefix/otherpkg/Otherlib/index.html), so it matched. But for pages, the page name is in the filename (e.g., prefix/otherpkg/otherpage.html), leaving only prefix/otherpkg in the directory - which didn't match prefix/otherpkg/ due to the trailing slash.

Fix by including the filename in the path used for prefix matching. Also normalize remap entries at config creation time (ensure trailing slashes) rather than on each lookup.

🤖 Generated with Claude Code

Page links were not being remapped because the remap prefix check
only matched against the directory path. For modules, the module name
is part of the directory (e.g., prefix/otherpkg/Otherlib/index.html),
so it matched. But for pages, the page name is in the filename
(e.g., prefix/otherpkg/otherpage.html), leaving only prefix/otherpkg
in the directory - which didn't match prefix/otherpkg/ due to the
trailing slash.

Fix by including the filename in the path used for prefix matching.
Also normalize remap entries at config creation time (ensure trailing
slashes) rather than on each lookup.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
}

let ensure_trailing_slash s =
if Astring.String.is_suffix ~affix:"/" s then s else s ^ "/"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think a possibly long list of string with requirements in them is the right data structure.
I think a Trie structure would be better.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, though this PR doesn't change that. It'll be a good future improvement.

Comment on lines +30 to +31
String.sub path (String.length prefix)
(String.length path - String.length prefix)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is also Astring.String.with_range ~first:(String.length prefix) path.

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.

2 participants