Re: [PATCHv2 1/2] git p4: Fixing script editor checks

[Luke Diamand <luke@diamand.org>]

On Wed, Apr 11, 2012 at 9:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Luke Diamand <luke@diamand.org> writes:
>
>> On Wed, Apr 11, 2012 at 7:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Luke Diamand <luke@diamand.org> writes:
>>>
>>>> If P4EDITOR is defined, the tests will fail when "git p4" starts an
>>>> editor.
>>>
>>> Is that a problem specific to tests, or should "git p4" itself unset that
>>> environment? If it is a problem specific to tests, would it be a better
>>> fix to add "P4EDITOR=:" like we do for EDITOR in t/test-lib.sh?
>>
>> Yes and no - git-p4.py will run $P4EDITOR if it is set, even if it's
>> just empty. So it would need a small fix to check for an empty string.
>> I can submit a suitable patch.
>
> How does real "p4" run $P4EDITOR?  For example, if you have:
>
>        P4EDITOR='vi -e'
>
> does it start "vi" in its "ex" mode, implying that it behaves as if the
> invocation were like this in a hypothetical implementation of "p4" as a
> shell script:
>
>        #!/bin/sh
>        ... do some stuff p4 does ...
>        $P4EDITOR file-to-be-edited
>
> Whatever it does needs to be emulated by the code in git-p4.py that runs
> it, as that is the way the end users expect, and if it turns out to be
> like the above, setting P4EDITOR to ':' just like we do to EDITOR in
> t/test-lib.sh should be the right thing to do.  The always-true command
> colon will leave the file-to-be-edited unmodified and report success.
>
> Similarly, what happens with "p4" when $P4EDITOR is set to an empty
> string?  If it ignores and falls back to the other usual way to find your
> preferred editor, you should emulate that as well in git-p4.py.
>
> Thanks.

"vi -e" => vi in ex mode
":" => always-true-command
"" => falls back to $EDITOR, and then to vi(UNIX or Win32 if $SHELL is
set) or notepad(Win32).