From: Phillip Wood <phillip.wood123@gmail.com>
To: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>,
"James Duley" <jagduley@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
James Duley via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org,
Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH] Ensure restore_term works correctly with DUPLEX
Date: Mon, 23 Jun 2025 10:42:12 +0100 [thread overview]
Message-ID: <d6e22dba-637b-4b5d-bd88-25fa320e22ff@gmail.com> (raw)
In-Reply-To: <ckzjxolc5jthge62zcm5abnssefwntc3ryu6sumpvrdontmyyq@d4ahvg5umked>
On 19/06/2025 00:43, Carlo Marcelo Arenas Belón wrote:
> On Thu, Jun 19, 2025 at 08:38:29AM -0800, James Duley wrote:
>> On Wed, 18 Jun 2025 at 22:07, Carlo Marcelo Arenas Belón
>> <carenas@gmail.com> wrote:
>>>
>> I thought about something like that, but I figured:
>> * restore_term is only called if save_term is successful
>> * hconout is always invalid before save_term is called
>
> but it is not always set to invalid at the end of restore_term()
> so it can't be relied upon either.
Are you sure about that? It looks to me like the only time we don't
reset hconout is when we use stty in which case I think hconout is
already set to the invalid handle value.
>> * 0 might be a valid cmode_out that should be restored
>
> cmode_out == 0 is not a valid mode that should be restored,
cmode_out is a bitfield. The documentation for SetConsoleMode() says the
mode parameter "can be one or more of the following values" which
implies zero is not a valid mode. It seems very hacky to be relying on
that when we can just check if the output handle is valid though.
> and
> indeed the original code was (ab)using that fact to decide if
> SetConsoleMode(hcounout) would be called at all (as a proxy to
> know if DUPLEX was used or not), hence why it is a bug not to
> update it, as you pointed out and found unexpectally, sorry
> about that.
>
>> This is my first patch so I didn't realize git-for-windows had a
>> separate fork. That makes sense now because I couldn't find where
>> save_term was called from in this repo. To test this works I had
>> downloaded the artifacts from
>> https://github.com/git/git/actions/runs/15692373534/job/44210362705
>> but is that right? If I should submit this patch to the git-for-windows
>> fork, please let me know. Or, if someone, who knows what they're
>> doing, wants to pick this up, they're more than welcome.
>
> You are going to need to get the Git for Windows SDK installed to
> be able to apply this patch and build your own version of GfW.
>
> IMHO getting the change that makes "cmode_out" reliable again (which
> would include both our changes) should be a good start regardless,
> and at least that change could be submitted here.
Yes, this change should absolutely be submitted here as it is fixing
code that is upstream of git-for-windows.
Thanks
Phillip
>
> Carlo
>
next prev parent reply other threads:[~2025-06-23 9:42 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-17 18:56 [PATCH] Ensure restore_term works correctly with DUPLEX James Duley via GitGitGadget
2025-06-17 19:18 ` Junio C Hamano
2025-06-18 10:07 ` Carlo Marcelo Arenas Belón
2025-06-18 20:38 ` James Duley
2025-06-18 23:43 ` Carlo Marcelo Arenas Belón
2025-06-23 9:42 ` Phillip Wood [this message]
2025-06-23 9:35 ` Phillip Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d6e22dba-637b-4b5d-bd88-25fa320e22ff@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=carenas@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=jagduley@gmail.com \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox