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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Feb 1, 2017, 22:42 UTC
Message-ID: <alpine.DEB.2.20.1702012339080.3496@virtualbox>
In-Reply-To: <xmqqzii5mthp.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Wed, 1 Feb 2017, Junio C Hamano wrote:

Show 14 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > That leaves the "putty" case in handle_ssh_variant(), does it not? Was it
> > not your specific objection that that is the case?
> 
> Yup, you can remove that while you reroll.
> 
> > No worries, I will let this simmer for a while. Your fixup has a lot of
> > duplicated code (so much for maintainability as an important goal... ;-))
> > and I will have to think about it. My immediate thinking is to *not*
> > duplicate code,...
> 
> You need to realize that the namespaces of the configuration and the
> command names are distinct.  There is no code duplication.

Since your 2/4 did away with the "plink" and "tortoiseplink" values in favor of "port_option" and "batch_option", there is a duplication of logic which I tried to undo in v3 and which you reintroduce in your fixup.

Maybe that refactoring was not so smart to begin with, and we should have instead modified the code to use an enum instead and keep the original conditionals instead of setting a port_option and a batch_option explicitly.

Ciao, Johannes