search

jgazeau / shadocs

Shadocs Theme for Hugo
https://jgazeau.github.io/shadocs/
MIT License
51 stars 20 forks source link

[feature_request]: Revise link resolution #73

Closed Anaminus closed 2 years ago

Anaminus commented 2 years ago

What is missing in the theme?

When parsing markdown links, Shadocs will use the relLangURL function to resolve URLs. A problem with this is that the function does not handle links relative to the current page. relref might be used instead, but it does not correctly handle external links.

It appears that the convention in Hugo is to leave links rendered from markdown as-is, and use the ref/relref shortcodes when linking to other pages on the website. Shadocs seems to go against this convention by parsing and handling links rendered by markdown, which is admittedly rather convenient. However, because relLangURL is used to resolve links, the resulting URL is naively relative to the configured baseURL, making it unsuitable for relative paths. The relref function is smarter about this, but it doesn't work with external links. Neither function can be used on its own.

To resolve this problem, the link renderer could parse the URL. If it has a scheme, then relLangURL is used as before. Otherwise, relref is used. To implement this for my website, I made the following change:

https://github.com/Anaminus/shadocs/commit/7982a49fb9a8a588475c2fc9059c63182c4a977f

I'm hesitant to submit this as a Pull Request because links with relative paths are likely to be interpreted differently. Accepting this change would involve users reevaluating all the links on their website, to make sure they are corrected for the new behavior. The effect is wide-reaching enough that it could be considered a breaking change.

jgazeau commented 2 years ago

Indeed, I did not tests all the behaviors regarding links. Actually I know that in other projects I used to actually have a common behavior when writing links (whether internal or external we always use the markdown syntax, instead of relying on the ref relref shortcode), and honestly it feels more easy for users and also for any processing to have just one way to define links. But in fact it will imply to manage it inside the link render hook. I'll try to make it ASAIC, and also enhance the tests to be exhaustive and test all possible cases:

jgazeau commented 2 years ago

Hello @Anaminus ,

Sorry for the delay, actually don't know what I did about the links but indeed I was completely mistaken 🤨. I've changed it and you can have a look at the updated preview documentation where I put all the link examples. It will indeed implied that users might have to revise their website links, but in any case the link management was not correct at all.