Subject: Re: [PATCH 06/14] remote.h: add get_remote_capabilities, request_capabilities

From: David Turner <[email-masked]>
Date: Mon, 02 May 2016 14:57:43 -0400

On Fri, 2016-04-29 at 16:34 -0700, Stefan Beller wrote:
 
> +const char *known_capabilities[] = {
> +	"multi_ack",
> +	"thin-pack",
> +	"side-band",
> +	"side-band-64k",
> +	"ofs-delta",
> +	"shallow",
> +	"no-progress",
> +	"include-tag",
> +	"multi_ack_detailed",
> +	"allow-tip-sha1-in-want",
> +	"allow-reachable-sha1-in-want",
> +	"no-done",
> +};

I wonder if it is possible to not repeat the list from upload-pack.c?
It seems unfortunate to have to add the same string in two places
whenever you add a capability.

> +static int keep_capability(char *line)

s/keep_/is_known_/ ?  Also it would be good to handle capabilities that
are prefixes of others correctly.

> +{
> +	int i;
> +	for (i = 0; i < ARRAY_SIZE(known_capabilities); i++)
> +		if (starts_with(line, known_capabilities[i]))
> +			return 1;
> +	return 0;
> +}
> +
> +void get_remote_capabilities(int in, char *src_buf, size_t src_len)

maybe rename "in" to "fd" or "in_fd"?  I don't immediately know what
"in" is supposed to be when I just look at this signature.

> +void request_capabilities(int out, struct string_list *list)

Maybe name this "send_capabilities_request"?