On Wed, Aug 05, 2015 at 10:34:34AM -0700, Junio C Hamano wrote: > > As you can see, there is a lot of complexity in there and I'm not > > convinced this is better than just exposing > > 'parse_connect_url()', which already handles everything for us. > > If the function "handles everything for us", that's fine, but the > primary reason I am hesitant is because parse_connect_url() was > designed specifically not to have to worry about some protocols > (e.g. I think feeding it a "http://"; would fail, and more > importantly, its current callers want such a call to fail). Also it > is meant to handle some non-protocols (e.g. scp style host:path that > does not follow <scheme>://...). True, but the transport code _is_ handling that at some point. It makes me wonder if it would be possible to push the call to transport_get further up inside cmd_clone(), and then provide some way to query the remote path and hostname from the transport code. Then guess_dir_name could just go away entirely, in favor of something like: dir_name = transport_get_path(transport); if (!*dir_name) dir_name = transport_get_host(transport); That may be overly simplistic or unworkable, though. I haven't dug into the code. > Also does it handle the "2222" case above? I do not think > parse_connect_url() even calls get_host_and_port() to be able to > tell what "2222" means in these examples. Speaking of which, has anyone tested whether the old or new code handles external remote helpers? Certainly: foo::https://host/repo.git should still use repo.git. But technically the string handed to git-remote-foo does not have to look anything like a URL. In those cases neither guess_dir_name nor the transport code have any idea what anything to the right of the "::" means; we probably have to resort to blind guessing based on characters like colon and slash. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html