icyleaf / markd

Yet another markdown parser, Compliant to CommonMark specification, written in Crystal.
MIT License
109 stars 31 forks source link

Enhance base URL resolution #27

Closed straight-shoota closed 4 years ago

straight-shoota commented 4 years ago

Turns out #26 wasn't sufficient for my use case. For https://shardbox.org I want to resolve relative links in repo readmes, relative to the repo URL. But GitHub for example uses different base URLs depending on whether it points to a raw resource (image) or the online HTML page (link).

In order to achieve that, I'd like to pass the current node as an argument to resolve_uri to convey the context. For my use case that would be sufficient. I can simply use that method to hook into the renderer and add my custom implementation.

I'm not sure whether this use case is common enough to have that behaviour included in Makrd directly. I'm not even sure about the original base_url option.

So there are three options:

  1. Only add node argument to resolve_uri, keep its default implementation and Options#base_url. That's the minimal change solution.
  2. Add node argument to resolve_uri but remove Options#base_url and make its default implementation to just return destination. That essentially reverts half of #26 and leaves implementation of base URL entirely to the user. That's the overall minimum intrusion solution.
  3. Add node argument to resolve_uri and add separate properties to Options: #base_url_link and #base_url_image which will be selected depending on the node. That's the all-inclusive solution. Not sure if it's too specific.

I can send a PR for either option. Let me know what you think.

icyleaf commented 4 years ago

This is not a common issue but we can achieve it first. the best solution is make it extensible, like gfm formatted support, etc.

For your case i choose option 1 or 2. PR is welcome, i have not much time to write Crystal for a while.

straight-shoota commented 4 years ago

I went with no 1 because I think a generic base_url option could probably be useful in other scenarios.

straight-shoota commented 4 years ago

Just a heads up: Unfortunately the solution presented here is insufficient because it only affects URLs in Markup code, but not when written in plain HTML. So [link](foo) is properly resolved on the base url but <a href="foo">link</a> would be left as is. The only way to solve this is to parse the resulting HTML code and apply base url resolution on href and src arguments.

The base_url option might still have it's use cases, I'm not sure. Maybe it would be better to remove it though :/