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

support zotero:// external link #8692

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zorrox1024
Copy link
Contributor

No description provided.

Copy link

@zorrox1024 It appears that this is your first contribution to the project, welcome.

With apologies for the bureaucracy, please could you prepare a separate PR to the 'tiddlywiki-com' branch with your signature for the Contributor License Agreement (see contributing.md).

@linonetwo
Copy link
Contributor

Should we just use \s+: or [^:]+: here?

@Leilei332
Copy link
Contributor

IMO the regexp can be changed to recognize all url schemes as external links.

Copy link

zorrox1024 has signed the Contributor License Agreement (see contributing.md)

@pmario
Copy link
Member

pmario commented Oct 20, 2024

IMO the regexp can be changed to recognize all url schemes as external links.

I do not think it should identify htp://something as an external link, because it is a typo.

There are about 300 official URL schemes and only a hand full of them make sense for TW.


@zorrox1024 zotero file scheme is not registered at IANA. So where can we find the specification?

@Jermolene
Copy link
Member

Thanks @zorrox1024. I think it is reasonable for users to want Zotero links to work within TiddlyWiki.

I wonder whether we might take this as an opportunity to refactor the external links mechanism so that plugins can extend it with additional URL schemes specified via configuration tiddlers. The core would then check through a list of external link regexps, and return "true" if any of them match. The implementation would need to be careful to maintain efficiency.

@Leilei332
Copy link
Contributor

Leilei332 commented Oct 20, 2024

Actually not all protocols are registered in IANA, the obsidian protocal is supported by tiddlywiki, though it does not exist in IANA. It's situation is like mimetypes, where many mimetypes (like text/org) is supported by Linux but isn't registered in IANA.

@zorrox1024
Copy link
Contributor Author

Should we just use \s+: or [^:]+: here?

I prefer to this solution. I should depends on browser or Desktop OS setting, which schema is supported.

for example, i custom a telnet:// scheme to start a telnet terminal to open connect fast

@pmario
Copy link
Member

pmario commented Oct 20, 2024

@Leilei332

That's right. But obsidian's spec is easy to find: https://help.obsidian.md/Extending+Obsidian/Obsidian+URI

@Jermolene
Copy link
Member

Should we just use \s+: or [^:]+: here?

I prefer to this solution. I should depends on browser or Desktop OS setting, which schema is supported.

Sadly I think there are too many false positives. The following strings all qualify as external URLs with the regexp /^[^:]+:[^\s<>{}\[\]|"\^]+(?:/|\b)/I`.

no:ah:thing
no:)3
no-:)3
-:s:

@zorrox1024
Copy link
Contributor Author

how about something like

/^[a-zA-Z][a-zA-Z\d+\-.]*:\/\//i

@Jermolene
Copy link
Member

I did some research on common URL schemes. For example, here's a list of schemes used by apps on macOS.

I think part of the problem is that our current URL regex is too lax. It doesn't require the sequence :// but a more complex pattern that matches that and other sequences. The challenge is that the vast majority of schemes have a double backslash, except for mailto which doesn't use slashes.

So, I wonder if we might simplify things by testing explicitly for ://, with some kind of workaround to deal with mailto:.

@linonetwo
Copy link
Contributor

linonetwo commented Oct 21, 2024

zotero file scheme is not registered at IANA. So where can we find the specification?

@pmario Desktop app can register anything, like tidgi://, it can be any string (probably without :/)

截屏2024-10-21 17 23 00

BTW I wanted to add support for tidgi:// here, if use regex here then I won't need to.

if we might simplify things by testing explicitly for ://, with some kind of workaround to deal with mailto:.

@zorrox1024 This is JS, so you can do anything, feel free, even use 2 regex and && them.

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.

6 participants