Patrick Steinhardt <ps@xxxxxx> writes: > - The naive way of just adding '@' as path separator would break > cloning repositories like '/foo/bar@xxxxxxx' (which would > currently become 'bar@baz' but would become 'baz' only). > > - Skipping the scheme initially is required because without it we > wouldn't be able to scan until the next dir separator in the > host part when stripping authentication information. > > - First checking for '/' in the current stripped URI when we > want to remove the port is required because we do not want to > strip port numbers when cloning from something like > '/foo/bar:2222.git' (which would currently become '2222' but > would then be stripped of the ':2222' part and instead become > 'bar'). Still, this breaks on cloning a bare repository in the > current dir (e.g. cloning 'bar:2222.git', which should become > '2222' because it is not a port number but would become > 'bar'). This is a very good write-up. Please make sure all of the above appears in the commit log message somewhere. > 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>://...). 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. > Maybe I'm just being blind for the obvious solution here, though. > > Patrick Steinhardt (6): > tests: fix broken && chains in t1509-root-worktree > tests: fix cleanup after tests in t1509-root-worktree > clone: do not include authentication data in guessed dir > clone: do not use port number as dir name > clone: abort if no dir name could be guessed > clone: add tests for cloning with empty path > > builtin/clone.c | 67 ++++++++++++++++++++++++++++++++++++++++-------- > t/t1509-root-worktree.sh | 51 +++++++++++++++++++++++++++++++++--- > 2 files changed, 103 insertions(+), 15 deletions(-) -- 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