-
Notifications
You must be signed in to change notification settings - Fork 15
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 replacement #95
base: main
Are you sure you want to change the base?
Conversation
Sorry it looks like there may be an edge case where this fails, checking if it's cache.. |
Oh, I ran into a separate related issue, worried it was my changes that did it 😂 On https://unjs.io/packages/changelogen see the link in the highlighted text, the replacement function replaced the URL string instead of the URL itself, I'll try and fix that as well 💪. |
I see now that removing those is intentional, so all good! |
@BobbieGoede changes look good, I must have missed since in the previous PR since I was trying to fix just relative images. @pi0 The only thing should we unit test of the helper function to cover most cases? |
Unit test would be nice! (also good for later refactors to move to an AST aware transform) |
I just added very basic unit tests for this function and changed the ci file to run the tests, coverage only checks |
Thanks @BobbieGoede looks great. |
I figured I would try my hand at a refactor using AST to replace the links as hinted at, I kept it on a separate branch here BobbieGoede#3. No pressure to use those changes, it's primarily for me to get more experience using ASTs 😄 |
Co-authored-by: Pooya Parsa <[email protected]>
8428228
to
63b0117
Compare
Resolves #94
This changes the relative link replacement, only image asset URLs are replaced/prefixed with the raw Github URL, while relative markdown URLs are simply replaced/prefixed with a normal (absolute) link to the branch tree.
Code is a bit messier with my changes 😅 happy to receive feedback.