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

Trailing punctuation is included in link (periods, question marks) #26

Closed
karlhorky opened this issue Aug 15, 2019 · 4 comments · Fixed by #49
Closed

Trailing punctuation is included in link (periods, question marks) #26

karlhorky opened this issue Aug 15, 2019 · 4 comments · Fixed by #49

Comments

@karlhorky
Copy link
Contributor

refined-github/refined-github#381 introduced adding links to URLs in code, which is a really cool feature!

However, it doesn't follow the same rules as Markdown in Markdown code, such as the case where a link has a period at the end (the link on https://github.com/palmerhq/tsdx/pull/138/files goes to https://github.com/alexreardon/tiny-invariant., with the period):

Screen Shot 2019-08-15 at 17 12 18

I can think of two ways to deal with this off the top of my head (maybe there's a better / easier alternative):

  • use a different regular expression in Markdown files that more closely matches Markdown semantics
  • don't allow periods at the last character of a URL? I haven't seen many (any?) URLs like this, and I think the number of URLs to be fixed by this is probably a lot higher...
@kidonng
Copy link

kidonng commented Aug 15, 2019

The regular expression comes from upstream and I think this is expected. There are times when links end with . (e.g. https://www.google.com.). Let alone it could end with other marks like ! :. Better practice is to add a space after the link 😀

@karlhorky
Copy link
Contributor Author

karlhorky commented Aug 15, 2019

Better practice is to add a space after the link

I don't think this is actually an argument. If it's just your code or my code, we can discuss this, sure. But we're talking about 3rd-party code on all historical commits on GitHub - can't go around asking people to rewrite their old commits.

Would be interesting to hear more real-world examples for links with punctuation at the end. I'm guessing there aren't many.

I think it's about what would happen more in real-world situations - links with punctuation at the end (which happen... almost never?) or instances of people adding punctuation at the end of links. I would guess there are tons of instances of the second case. So the tradeoff would be to break the URLs in the almost-never case of links with punctuation at the end in order to fix something that happens a lot.

The regular expression comes from upstream

I guess I would probably not suggest PRing this to upstream, since that module does its job nicely and does not have the constraints that refined-github does. I would suggest putting in additional manipulation after the transformation from the upstream module to fix this issue.

cc @sindresorhus because ownership of upstream.

@fregante
Copy link
Collaborator

I asked @sindresorhus to move this issue there. It won't be fixed here, especially because technically we're still waiting for Firefox to update to linkify-urls@3. Info in sindresorhus/refined-github#803

@karlhorky
Copy link
Contributor Author

Ok, if it should be an issue on upstream, then maybe there would be a way it could make sense - a default-off option like stripTrailingPunctuation, for example.

@sindresorhus sindresorhus transferred this issue from refined-github/refined-github Aug 16, 2019
@fregante fregante changed the title URL Links with Period at End Fail in Markdown Trailing punctuation is included in link (periods, question marks) Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants