From: Phillip Wood <phillip.wood123@gmail.com>
Date: Thu, 10 Mar 2022 18:18:31 GMT
Subject: Re: [PATCH v2 0/4] builtin add -p: hopefully final readkey fixes
Message-ID: <a1e51433-71b1-b32d-0475-192b83bac43a@gmail.com>
In-Reply-To: <93d197db-c52c-101b-bdb0-3b4c9b073705@gmail.com>

On 10/03/2022 13:28, Phillip Wood wrote:
> Hi Junio
> 
> On 09/03/2022 23:37, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> $ git checkout --detach pw/single-key-interactive
>>> HEAD is now at ac618c418e add -p: disable stdin buffering when 
>>> interactive.singlekey is set
>>> $ git am -s ./+pw4-v2-add-p-single
>>> Applying: terminal: use flags for save_term()
>>> Applying: terminal: don't assume stdin is /dev/tty
>>> Applying: terminal: work around macos poll() bug
>>> error: patch failed: compat/terminal.c:397
>>> error: compat/terminal.c: patch does not apply
>>> Patch failed at 0003 terminal: work around macos poll() bug
>>> hint: Use 'git am --show-current-patch=diff' to see the failed patch
>>> When you have resolved this problem, run "git am --continue".
>>> If you prefer to skip this patch, run "git am --skip" instead.
>>> To restore the original branch and stop patching, run "git am --abort".
>>> $ exit
>>
>> I think I figured it out.  A merge of pw/single-key-interactive into
>> a recent tip of 'master' wants the "return 0" in the preimage below
>> to be "break" in compat/terminal.c
>>
>>
>>> @@ -397,12 +433,7 @@ int read_key_without_echo(struct strbuf *buf)
>>>            * half a second when we know that the sequence is complete.
>>>            */
>>>           while (!is_known_escape_sequence(buf->buf)) {
>>> -            struct pollfd pfd = { .fd = 0, .events = POLLIN };
>>> -
>>> -            if (poll(&pfd, 1, 500) < 1)
>>> -                break;
>>> -
>>> -            ch = getchar();
>>> +            ch = getchar_with_timeout(500);
>>>               if (ch == EOF)
>>>                   return 0;

Looking more closely that should be "break" not "return 0". I think what 
has happened is that I accidentally based these on an old version of 
pw/single-key-interactive which did not contain 24d7ce383a ("terminal: 
always reset terminal when reading without echo", 2022-02-22)

Sorry for the confusion

Phillip