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

Pkldoc generates broken page anchors #395

Open
translatenix opened this issue Apr 4, 2024 · 3 comments
Open

Pkldoc generates broken page anchors #395

translatenix opened this issue Apr 4, 2024 · 3 comments

Comments

@translatenix
Copy link
Contributor

Searching the standard library docs, I noticed that I always land at the top of the page, not at the function I search for. For example, searching for sort takes me to https://pkl-lang.org/package-docs/pkl/current/base/Collection.html#sort(). But #sort() doesn't exist, so I land at the top of the page. Inspecting the page source, I found that id attributes are erroneously percent-encoded:

<div id="sort%28%29" class="anchor">
@bioball
Copy link
Contributor

bioball commented Apr 4, 2024

Thanks for the error report.

Although, it's not strictly necessary, the percent-encoding there is intentional (we are using java.net.URLEncoder underneath the hood to encode links and anchors).

We should similarly run links from the generated search index through URLEncoder.

@translatenix
Copy link
Contributor Author

translatenix commented Apr 4, 2024

Although, it's not strictly necessary, the percent-encoding there is intentional (we are using java.net.URLEncoder underneath the hood to encode links and anchors).
We should similarly run links from the generated search index through URLEncoder.

Unnecessarily obfuscating Pkldoc URLs will result in a worse user experience.
Swift and Scala API docs are fine with parentheses in URL paths/fragments, as is the URL spec.

https://developer.apple.com/documentation/swift/array/sorted(by:)
https://www.scala-lang.org/api/2.13.5/scala/collection/SeqOps.html#sortWith(lt:(A,A)=>Boolean):C
https://url.spec.whatwg.org/#path-percent-encode-set
https://url.spec.whatwg.org/#fragment-percent-encode-set

From https://docs.rs/crate/percent-encoding/2.3.1 (emphasis mine):

When encoding, the set of characters that can (and should, for readability) be left alone depends on the context.

@bioball
Copy link
Contributor

bioball commented Apr 4, 2024

Yup, it definitely doesn't need to encode parentheses. It'd certainly be an improvement to not encode them here.

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

No branches or pull requests

2 participants