Handle protocols that seperate the host from path with colon #3471

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
6 participants

Fixes #1627. Added tests too.

@bnoordhuis bnoordhuis commented on the diff Jun 28, 2012

lib/url.js
'http:': true,
'https:': true,
'ftp:': true,
'gopher:': true,
- 'file:': true
+ 'file:': true,
+ 'ssh:': true
@bnoordhuis

bnoordhuis Jun 28, 2012

Owner

Unnecessary change, please undo.

Owner

bnoordhuis commented Jun 28, 2012

@isaacs You should probably review this. I don't know if this is what you had in mind when you filed #1627.

The problem I see with it is that it's a hard-coded protocol check. It breaks down when people use mashup URLs like 'git+ssh://example.com:path/to/foo'.

@bnoordhuis @isaacs and me talked about it in IRC and he gave me some feedback. I am going to modify this soon.

isaacs commented Jun 29, 2012

@bnoordhuis Yes, I want to +1 this before landing. Thanks.

@pksunkara Feel free to continue to discuss in IRC and elsewhere :)

@pksunkara, I'd hate to see your work for naught. Thinking of providing an update? =)

Can one of the admins verify this patch?

hackedy commented Apr 20, 2013

@pksunkara has deleted their fork so this particular pull can get closed. I am going to open a PR in a minute with this fix, though

isaacs commented Apr 23, 2013

@hackedy If you do, can you please reference #3471 in the comments, so that we have continuity? Thanks.

isaacs closed this Apr 23, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment