Re: [PATCH v2 3/3] connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 27, 2017, 10:35 UTC
Message-ID: <alpine.DEB.2.20.1701271132270.3469@virtualbox>
In-Reply-To: <xmqqo9yt4o5i.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Thu, 26 Jan 2017, Junio C Hamano wrote:

Show 30 quoted lines

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > diff --git a/connect.c b/connect.c
> > index 9f750eacb6..7b4437578b 100644
> > --- a/connect.c
> > +++ b/connect.c
> > @@ -691,6 +691,24 @@ static const char *get_ssh_command(void)
> >  	return NULL;
> >  }
> >  
> > +static int handle_ssh_variant(int *port_option, int *needs_batch)
> > +{
> > +	const char *variant;
> > +
> > +	if (!(variant = getenv("GIT_SSH_VARIANT")) &&
> > +		git_config_get_string_const("ssh.variant", &variant))
> > +		return 0;
> > +
> > +	if (!strcmp(variant, "plink") || !strcmp(variant, "putty"))
> > +		*port_option = 'P';
> > +	else if (!strcmp(variant, "tortoiseplink")) {
> > +		*port_option = 'P';
> > +		*needs_batch = 1;
> > +	}
> > +
> > +	return 1;
> > +}
> 
> Between handle and get I do not think there is strong reason to
> favor one over the other.

That is correct. "handle" and "get" are two very beautiful words, and none of them deserves to take a back seat behind the other.

In this case, "get" is inappropriate, though, as the function does not return the ssh variant, nor does it assign the ssh variant to any variable to which any of its argument points.

Will try to find the time to address the other issues soon, Johannes