search

sindresorhus / linkify-urls

Linkify URLs in a string
MIT License
160 stars 22 forks source link

Fix `*` and `$` not being linkified #36

Closed rocktimsaikia closed 3 years ago

rocktimsaikia commented 3 years ago

fixes #33 Fixes the issue of asterisks not being linkified properly. Added test for it too.

//@sindresorhus

sindresorhus commented 3 years ago

* is reserved and should have been percentage encoded: https://en.wikipedia.org/wiki/Percent-encoding#:~:text=Reserved%20characters%20are%20those%20characters,characters%20have%20no%20such%20meanings.

rocktimsaikia commented 3 years ago

@sindresorhus Ah I see. So doesn't that mean all the reserved characters should be percentage-encoded? Cause currently, none of the reserved characters are getting url-encoded.

sindresorhus commented 3 years ago

Hmm, seems to be conflicting info about this. The RFC actually allows unencoded *: https://stackoverflow.com/a/6533595/64949

sindresorhus commented 3 years ago

Can you make sure all the mentioned characters here are supported unencoded?

Thus, only alphanumerics, the special characters $-_.+!*'(), and reserved characters used for their reserved purposes may be used unencoded within a URL.

rocktimsaikia commented 3 years ago

@sindresorhus Yes those characters are supported unencoded as stated on RFC. But still It does seem to have some conflicting views on this. May be cause most of us are not actually informed about this. So I guess we can just leave it as unencoded and mention about this information on the Readme properly so that others can know about it too.

sindresorhus commented 3 years ago

So I guess we can just leave it as unencoded and mention about this information on the Readme properly so that others can know about it too.

I'm not sure what you're saying here, but I'm saying I'm ok with this PR if you ensure all the mentioned characters are supported.

rocktimsaikia commented 3 years ago

@sindresorhus Ah sorry I misunderstood it. I checked and the dollar sign $ was not supported too. I added it as well along with a test for it too.