Re: [PATCH 0/2] Feature: New Variable git-p4.binary

[Luke Diamand <luke@diamand.org>]

On Thu, 14 Nov 2019 at 02:36, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Ben Keene via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > Issue: Using git-p4.py on Windows does not resolve properly to the p4.exe
> > binary in all instances.
> >
> > Two new code features are added to resolve the p4 executable location:
> >
> >  1. A new variable, git-p4.binary, has been added that takes precedence over
> >     the default p4 executable name. If this git option is set and the
> >     path.exists() passes for this file it will be used as executable for the
> >     system.popen calls.
> >
> >
> >  2. If the new variable git-p4.binary is not set, the program checks if the
> >     operating system is Windows. If it is, the executable is changed to
> >     'p4.exe'. All other operating systems
> >     (those that do not report 'Windows' in the platform.system() call)
> >     continue to use the current executable of 'p4'.
>
> I do not use Windows nor git-p4, but the above two changes make
> sense to me at the design level.  One thing that needs to be updated
> is the configuration variable, though.  It is more in line with the
> other parts of the system to name this "git-p4.program", because
> binary-ness does not matter much to you, as long as the named thing
> works correctly as "p4" program replacement.
>
> Seeing "gpg.program" is used in a similar way, it also is tempting
> to call it "p4.program", but no other parts of Git other than
> "git-p4" uses P4, and the "git-p4." prefix is to collect variables
> related to "git-p4" together, so calling it "p4.program" may not be
> a good idea---it would hurt discoverability.  "git-p4.p4program"
> may be OK, if we anticipate that git-p4 may need to use (and its
> users need to specify paths to) external programs other than "p4",
> but it probably is overkill.
>
> Thanks.

There's some trailing whitespace in 98bae, can we get that tidied up?

Otherwise, modulo Junio's comments, it looks good.

I agree that "git-p4.binary" might be harder to discover ("Oh, I
assumed that enabled binary mode!"). p4program should be fine.