Re: [PATCH v2] launch_editor(): indicate that Git waits for user input

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 20, 2017, 00:11 UTC
Message-ID: <xmqqvai5ollx.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <d53a655b-f51e-3f44-23db-581071010fd2@gmail.com>

Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:

Show 5 quoted lines

>>> However, it's not clear how much benefit you gain from stashing this
>>> away in a static variable. Premature optimization?
>>
>> The variable being "static" could be (but it was done primarily
>> because it allowed me not to worry about freeing),

The current code happens to be safe because I do not allocate. I do not know what others will do to the code in the future, and at that point, instinct kicks in to futureproof against the worst ;-).

Show 15 quoted lines

>>> Should printing of close_notice be done before the error()? Otherwise,
>>> you get this:
>>>
>>> --- 8< ---
>>> Launched your editor (...) ...There was a problem...
>>> --- 8< ---
>>
>> In my version with a far shorter message, I deliberately chose not
>> to clear the notice.  We ran the editor, and we saw a problem.  That
>> is what happened and that is what will be left on the terminal.
>>
>
> It might be a good thing to keep the notice but I think it would be
> better to have that error message in a "new line". I'm not sure if
> it's possible or not.

Of course it is possible, if you really wanted to. The code knows if it gave the "we launched and waiting for you" notice, so it can maintain not just one (i.e. "how I close the notice?") but another one (i.e. "how I do so upon an error?") and use it in the error codepath.