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

fix relative link parsing #136

Merged
merged 1 commit into from
Oct 13, 2022
Merged

Conversation

rajatjindal
Copy link
Contributor

still testing the changes out, for some reason unable to run make test on my laptop.

@rajatjindal
Copy link
Contributor Author

.md format href in rendered html
[URL](https://en.wikipedia.org/) <a href="https://en.wikipedia.org/">URL</a>
[relative link](./relative.md) <a href="relative">relative link</a>
[relative link](elsewhere.md) <a href="elsewhere">relative link</a>
[relative deep link](./relatively/deep.md) <a href="relatively/deep">relative deep link</a>
[absolute deep link](/absolute/deep.md) <a href="/absolute/deep">absolute deep link</a>

Signed-off-by: Rajat Jindal <[email protected]>
@rajatjindal
Copy link
Contributor Author

I will test more tomorrow to ensure it does not break anything.

Copy link
Contributor

@dicej dicej left a comment

Choose a reason for hiding this comment

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

Looks good to me. It's possible this might break links that were implicitly absolute, but we should be able to fix those on a case-by-case basis.

@rajatjindal
Copy link
Contributor Author

Thanks for looking at the pr. Could u pls give an example of the link you are referring to. I will add that to the testcase and verify that it works.

Many thanks
Rajat Jindal

@dicej
Copy link
Contributor

dicej commented Oct 12, 2022

For example, imagine you have a couple of documents like this:

foo/bar/baz.md
hello.md

And imagine foo/bar/baz.md has a link like [hello](./hello.md) in it. That's a bad link -- it should be [hello](../../hello.md) instead, but currently bartholomew will render it so it works because all links are converted to absolute. With your change, it will no longer work, but that's ok because it shouldn't have worked in the first place. So we just need to fix such cases in the .md files if we find them.

@rajatjindal
Copy link
Contributor Author

thanks for the explanation @dicej . yeah I would also consider that as bad link and that should be fixed in the content itself

@rajatjindal rajatjindal changed the title [WIP] fix relative link parsing fix relative link parsing Oct 13, 2022
@rajatjindal rajatjindal merged commit 519ff04 into fermyon:main Oct 13, 2022
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.

3 participants