search

sindresorhus / linkify-urls

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

Failing test for HTTP username #6

Closed fregante closed 6 years ago

fregante commented 7 years ago

Example (noticeable with Refined GitHub): https://github.com/scieloorg/opac_proc/blob/46d9f826d2519573b99418f11da1ce3f87bfa7eb/requirements.txt#L12-L14

Related: should it support git+http urls at all?

sindresorhus commented 7 years ago

Related: should it support git+http urls at all?

Is there really any point? Clicking it wouldn't work anyway.

fregante commented 7 years ago

@sindresorhus it's mostly about expectations. Also perhaps people have git URL handlers that open SourceTree for example.

sindresorhus commented 7 years ago

Who's' expectations though? I definitely wouldn't expect such URLs to be linkified. I actually don't want it at all, as it creates noise and gives a false expectation that they will work.

Also perhaps people have git URL handlers that open SourceTree for example.

That would be a minority, and everyone else would have to pay with useless links and wasted time clicking them and then realizing it's not working.

sindresorhus commented 7 years ago

I see that -e git+https://github.com/scieloorg/xylose@1.17.5#egg=xylose got linkified in your referenced example. I don't think we should linkify that either as it obviously doesn't work. In your linked case, it would make more sense for Octolinker to handle those.

fregante commented 7 years ago

How do we detect that though? The regex will have to look for the character that comes before http as well

sindresorhus commented 7 years ago

We would need a lookbehind regex pattern, which is still just a draft standard. Alternatively we could change it to also match git+ and then filter the URLs afterwards.

sindresorhus commented 6 years ago

@bfred-it Could you change the test to what we've discussed? (Not allowing + prefixed URLs)

fregante commented 6 years ago

Not sure of how to do that :D

sindresorhus commented 6 years ago

I'm only talking about the test, not the implementation. This is what I meant: https://github.com/sindresorhus/linkify-urls/commit/4db31ab79d62dc0adf6b3c086a6a68f955343918 :)

sindresorhus commented 6 years ago

Continued in https://github.com/sindresorhus/linkify-urls/issues/7

fregante commented 6 years ago

That’s unrelated to this PR, URLs with user:pass are still real HTTP and browsers can handle them

sindresorhus commented 6 years ago

Oops, I was too focused on the + stuff.