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

Update link to make them Obsidian compatible #28

Merged
merged 4 commits into from
Nov 28, 2023

Conversation

Portevent
Copy link
Contributor

fix #6

@berniexie
Copy link
Contributor

@Portevent typescript is failing, can you fix the errors please?

@berniexie
Copy link
Contributor

@Portevent so right now we can link either to an internal page or an external link. If we add your code in, I think external links break. I wonder if there's a way to see if the link matches an internal doc and if not, keep it as an external link?

@alixander
Copy link
Contributor

@berniexie why would external links break? this is just adding extra attributes, not changing any afaik.

But yes, there should be a way to identify external links: the presence of URL protocols. https://developer.mozilla.org/en-US/docs/Web/API/URL/protocol

What if users want to link to other resources though that are neither external nor Obsidian pages? Like a local PDF or image? /home/mycat.pdf. We should test that those work, if they were working before.

@berniexie
Copy link
Contributor

local links were not working before

@Portevent
Copy link
Contributor Author

@Portevent so right now we can link either to an internal page or an external link. If we add your code in, I think external links break. I wonder if there's a way to see if the link matches an internal doc and if not, keep it as an external link?

I don't think that would affect external link. But in case I added a check to skip external link. I made the assuption any external link follows the pattern <protocol>:\\<adress>, but we can change the regex if something is more suited.

Copy link
Contributor

@berniexie berniexie left a comment

Choose a reason for hiding this comment

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

@Portevent
Changes look good to me, made some small tweaks. However, the commit "Fix typescript & only apply modification to internal link" is unsigned though, can u fix that please

@korniychuk
Copy link

Really waiting for this PR delivered 🙏

Copy link
Contributor

@gavin-ts gavin-ts left a comment

Choose a reason for hiding this comment

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

rebased to resolve conflicts + sign the unsigned commit

@gavin-ts gavin-ts merged commit d4470fd into terrastruct:master Nov 28, 2023
2 checks passed
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.

Internal Links
5 participants