Re: [PATCH v5 4/9] connect: make parse_connect_url() return separated host and port

From: Junio C Hamano <gitster@pobox.com>
Date: May 16, 2016, 22:39 UTC
Message-ID: <xmqqr3d1zkz7.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160516000740.19042-5-mh@glandium.org>

Mike Hommey <mh@glandium.org> writes:

Show 7 quoted lines

> +	get_host_and_port(&host, &port);
> +
> +	if (*host && !port) {
> +		/* The host might contain a user:password string, ignore it
> +		 * when searching for the port again */
> +		char *end_user = strrchr(host, '@');
> +		port = get_port(end_user ? end_user : host);

Scanning from the right because host part would never have '@', but there could be an invalid URL with an unquoted '@' in userinfo part? Then this makes sense.

Show 10 quoted lines

> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index 91a69fc..9acba2b 100755
> --- a/t/t5500-fetch-pack.sh
> +++ b/t/t5500-fetch-pack.sh
> @@ -553,7 +553,7 @@ check_prot_path () {
>  	Diag: protocol=$2
>  	Diag: path=$3
>  	EOF
> -	git fetch-pack --diag-url "$1" | grep -v hostandport= >actual &&
> +	git fetch-pack --diag-url "$1" | grep -v host= | grep -v port= >actual &&

A single process:

	... | grep -v -e '^host=' -e '^port='

perhaps?

Show 26 quoted lines

> @@ -562,22 +562,17 @@ check_prot_host_port_path () {
>  	case "$2" in
>  		*ssh*)
>  		pp=ssh
> -		uah=userandhost
> -		ehost=$(echo $3 | tr -d "[]")
> -		diagport="Diag: port=$4"
>  		;;
>  		*)
> -		pp=$p
> -		uah=hostandport
> -		ehost=$(echo $3$4 | sed -e "s/22$/:22/" -e "s/NONE//")
> -		diagport=""
> +		pp=$2
>  		;;
>  	esac
> +	ehost=$(echo $3 | tr -d "[]")
>  	cat >exp <<-EOF &&
>  	Diag: url=$1
>  	Diag: protocol=$pp
> -	Diag: $uah=$ehost
> -	$diagport
> +	Diag: userandhost=$ehost
> +	Diag: port=$4
>  	Diag: path=$5
>  	EOF

This makes the diag output simpler and allows the caller to expect the same set of variables, which is good.