On 06/03/2016 01:13 AM, Mike Hommey wrote:
On Thu, Jun 02, 2016 at 03:52:41PM -0700, Junio C Hamano wrote:
* mh/connect (2016-06-01) 9 commits
- connect: move ssh command line preparation to a separate function
- connect: actively reject git:// urls with a user part
- connect: change the --diag-url output to separate user and host
- connect: make parse_connect_url() return the user part of the url as a separate value
- connect: group CONNECT_DIAG_URL handling code
- connect: make parse_connect_url() return separated host and port
- connect: re-derive a host:port string from the separate host and port variables
- connect: call get_host_and_port() earlier
- connect: document why we sometimes call get_port after get_host_and_port
Needs review.
I /think/ Torsten reviewed it all, and his last comments are in
$gmane/295800. It's still not clear to me why he wants to remove the
comment about [].
There where 2 comments in the review.
The most important thing is that now
git://[example.com:123]/path/to/repo is valid, but it shouldn't.
This patch fixes it:
@@ -673,7 +669,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_user,
* "host:port" and NULL.
* To support this undocumented legacy we still need to split the port.
*/
- if (!port)
+ if (!port && protocol == PROTO_SSH)
The other thing is that I asked for a test case for
git://[example.com:123]/path/to/repo
which shouldn't be hard to do.
If nobody else things that this comment in the code is stale:
- /*
- * Don't do destructive transforms as protocol code does
- * '[]' unwrapping in get_host_and_port()
- */
then just leave it as it is.
Thanks
--
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