* [PATCH 1/1] Cygwin: console: Maintain EXTENDED_FLAGS state
@ 2022-02-10 0:09 Mitchell Hentges
2022-02-10 8:07 ` Takashi Yano
0 siblings, 1 reply; 6+ messages in thread
From: Mitchell Hentges @ 2022-02-10 0:09 UTC (permalink / raw)
To: cygwin-patches
As well-described over in this post [1], it's possible for
the active console mode to be impossible to correctly determine.
Specifically, if ENABLE_EXTENDED_FLAGS is at any point unset,
then the flags it's associated with (ENABLE_INSERT_MODE,
ENABLE_QUICK_EDIT_MODE) will no longer be discoverable - they'll
always show up as unset, regardless of real console state.
It's not possible to work around this by setting
ENABLE_EXTENDED_FLAGS once then re-querying, because setting
ENABLE_EXTENDED_FLAGS on it's own will *disable* its related
flags.
Anyways, to avoid this case, all programs doing SetConsoleMode()
must be good community citizens and carefully maintain its state.
Unfortunately, we're accidentally stepping on this in
fhandler_console::set_input_mode().
This patch solves this by carrying forward ENABLED_EXTENDED_FLAGS
(and friends) in the only place where it had been ignored: set_input_mode()
Since the previous behaviour of leaving all three flags unset would
essentially maintain their existing state (except for the footgun
being worked around here), *adding* the carry-over of the flags now
should not alter console behaviour.
[1] https://www.os2museum.com/wp/disabling-quick-edit-mode/
---
winsup/cygwin/fhandler_console.cc | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/winsup/cygwin/fhandler_console.cc
b/winsup/cygwin/fhandler_console.cc
index 7a1a45bc1..b2554c3ba 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -458,16 +458,18 @@ void
fhandler_console::set_input_mode (tty::cons_mode m, const termios *t,
const handle_set_t *p)
{
- DWORD flags = 0, oflags;
+ DWORD oflags;
WaitForSingleObject (p->input_mutex, mutex_timeout);
GetConsoleMode (p->input_handle, &oflags);
+ DWORD flags = oflags
+ & (ENABLE_EXTENDED_FLAGS | ENABLE_INSERT_MODE |
ENABLE_QUICK_EDIT_MODE);
switch (m)
{
case tty::restore:
- flags = ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT |
ENABLE_PROCESSED_INPUT;
+ flags |= ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT |
ENABLE_PROCESSED_INPUT;
break;
case tty::cygwin:
- flags = ENABLE_WINDOW_INPUT;
+ flags |= ENABLE_WINDOW_INPUT;
if (wincap.has_con_24bit_colors () && !con_is_legacy)
flags |= ENABLE_VIRTUAL_TERMINAL_INPUT;
else
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Cygwin: console: Maintain EXTENDED_FLAGS state
2022-02-10 0:09 [PATCH 1/1] Cygwin: console: Maintain EXTENDED_FLAGS state Mitchell Hentges
@ 2022-02-10 8:07 ` Takashi Yano
2022-02-10 15:38 ` Mitchell Hentges
0 siblings, 1 reply; 6+ messages in thread
From: Takashi Yano @ 2022-02-10 8:07 UTC (permalink / raw)
To: cygwin-patches
On Wed, 9 Feb 2022 19:09:02 -0500
Mitchell Hentges wrote:
> As well-described over in this post [1], it's possible for
> the active console mode to be impossible to correctly determine.
> Specifically, if ENABLE_EXTENDED_FLAGS is at any point unset,
> then the flags it's associated with (ENABLE_INSERT_MODE,
> ENABLE_QUICK_EDIT_MODE) will no longer be discoverable - they'll
> always show up as unset, regardless of real console state.
>
> It's not possible to work around this by setting
> ENABLE_EXTENDED_FLAGS once then re-querying, because setting
> ENABLE_EXTENDED_FLAGS on it's own will *disable* its related
> flags.
>
> Anyways, to avoid this case, all programs doing SetConsoleMode()
> must be good community citizens and carefully maintain its state.
> Unfortunately, we're accidentally stepping on this in
> fhandler_console::set_input_mode().
>
> This patch solves this by carrying forward ENABLED_EXTENDED_FLAGS
> (and friends) in the only place where it had been ignored: set_input_mode()
> Since the previous behaviour of leaving all three flags unset would
> essentially maintain their existing state (except for the footgun
> being worked around here), *adding* the carry-over of the flags now
> should not alter console behaviour.
>
> [1] https://www.os2museum.com/wp/disabling-quick-edit-mode/
> ---
> winsup/cygwin/fhandler_console.cc | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler_console.cc
> b/winsup/cygwin/fhandler_console.cc
> index 7a1a45bc1..b2554c3ba 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> @@ -458,16 +458,18 @@ void
> fhandler_console::set_input_mode (tty::cons_mode m, const termios *t,
> const handle_set_t *p)
> {
> - DWORD flags = 0, oflags;
> + DWORD oflags;
> WaitForSingleObject (p->input_mutex, mutex_timeout);
> GetConsoleMode (p->input_handle, &oflags);
> + DWORD flags = oflags
> + & (ENABLE_EXTENDED_FLAGS | ENABLE_INSERT_MODE |
> ENABLE_QUICK_EDIT_MODE);
> switch (m)
> {
> case tty::restore:
> - flags = ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT |
> ENABLE_PROCESSED_INPUT;
> + flags |= ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT |
> ENABLE_PROCESSED_INPUT;
> break;
> case tty::cygwin:
> - flags = ENABLE_WINDOW_INPUT;
> + flags |= ENABLE_WINDOW_INPUT;
> if (wincap.has_con_24bit_colors () && !con_is_legacy)
> flags |= ENABLE_VIRTUAL_TERMINAL_INPUT;
> else
> --
> 2.35.1
Thanks for the patch. The patch looks good, however, it is
malformed, probably due to your mailer. (e.g. Sprious new
line inserted, and tab not preserved.)
Could you please resend it?
--
Takashi Yano <takashi.yano@nifty.ne.jp>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] Cygwin: console: Maintain EXTENDED_FLAGS state
2022-02-10 8:07 ` Takashi Yano
@ 2022-02-10 15:38 ` Mitchell Hentges
2022-02-10 15:40 ` Mitchell Hentges
0 siblings, 1 reply; 6+ messages in thread
From: Mitchell Hentges @ 2022-02-10 15:38 UTC (permalink / raw)
To: cygwin-patches
As well-described over in this post [1], it's possible for
the active console mode to be impossible to correctly determine.
Specifically, if ENABLE_EXTENDED_FLAGS is at any point unset,
then the flags it's associated with (ENABLE_INSERT_MODE,
ENABLE_QUICK_EDIT_MODE) will no longer be discoverable - they'll
always show up as unset, regardless of real console state.
It's not possible to work around this by setting
ENABLE_EXTENDED_FLAGS once then re-querying, because setting
ENABLE_EXTENDED_FLAGS on it's own will *disable* its related
flags.
Anyways, to avoid this case, all programs doing SetConsoleMode()
must be good community citizens and carefully maintain its state.
Unfortunately, we're accidentally stepping on this in
fhandler_console::set_input_mode().
This patch solves this by carrying forward ENABLED_EXTENDED_FLAGS
and friends in the only place where it had been ignoring it.
Since the previous behaviour of leaving all three flags unset would
essentially maintain their existing state (except for the footgun
being worked around here), *adding* the carry-over of the flags now
should not alter console behaviour.
---
winsup/cygwin/fhandler_console.cc | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 7a1a45bc1..b2554c3ba 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -458,16 +458,18 @@ void
fhandler_console::set_input_mode (tty::cons_mode m, const termios *t,
const handle_set_t *p)
{
- DWORD flags = 0, oflags;
+ DWORD oflags;
WaitForSingleObject (p->input_mutex, mutex_timeout);
GetConsoleMode (p->input_handle, &oflags);
+ DWORD flags = oflags
+ & (ENABLE_EXTENDED_FLAGS | ENABLE_INSERT_MODE | ENABLE_QUICK_EDIT_MODE);
switch (m)
{
case tty::restore:
- flags = ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT;
+ flags |= ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT;
break;
case tty::cygwin:
- flags = ENABLE_WINDOW_INPUT;
+ flags |= ENABLE_WINDOW_INPUT;
if (wincap.has_con_24bit_colors () && !con_is_legacy)
flags |= ENABLE_VIRTUAL_TERMINAL_INPUT;
else
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Cygwin: console: Maintain EXTENDED_FLAGS state
2022-02-10 15:38 ` Mitchell Hentges
@ 2022-02-10 15:40 ` Mitchell Hentges
2022-02-11 0:12 ` Takashi Yano
0 siblings, 1 reply; 6+ messages in thread
From: Mitchell Hentges @ 2022-02-10 15:40 UTC (permalink / raw)
To: cygwin-patches
Thanks, I appreciate it.
The initial send was via GMail, but I've wired up git-send-email to msmtp,
and I'm hoping that it's happy now - at least, it looks like tabs are being
preserved now, which is a good sign.
On Thu, Feb 10, 2022 at 10:38 AM Mitchell Hentges <mhentges@mozilla.com>
wrote:
> As well-described over in this post [1], it's possible for
> the active console mode to be impossible to correctly determine.
> Specifically, if ENABLE_EXTENDED_FLAGS is at any point unset,
> then the flags it's associated with (ENABLE_INSERT_MODE,
> ENABLE_QUICK_EDIT_MODE) will no longer be discoverable - they'll
> always show up as unset, regardless of real console state.
>
> It's not possible to work around this by setting
> ENABLE_EXTENDED_FLAGS once then re-querying, because setting
> ENABLE_EXTENDED_FLAGS on it's own will *disable* its related
> flags.
>
> Anyways, to avoid this case, all programs doing SetConsoleMode()
> must be good community citizens and carefully maintain its state.
> Unfortunately, we're accidentally stepping on this in
> fhandler_console::set_input_mode().
>
> This patch solves this by carrying forward ENABLED_EXTENDED_FLAGS
> and friends in the only place where it had been ignoring it.
> Since the previous behaviour of leaving all three flags unset would
> essentially maintain their existing state (except for the footgun
> being worked around here), *adding* the carry-over of the flags now
> should not alter console behaviour.
> ---
> winsup/cygwin/fhandler_console.cc | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler_console.cc
> b/winsup/cygwin/fhandler_console.cc
> index 7a1a45bc1..b2554c3ba 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> @@ -458,16 +458,18 @@ void
> fhandler_console::set_input_mode (tty::cons_mode m, const termios *t,
> const handle_set_t *p)
> {
> - DWORD flags = 0, oflags;
> + DWORD oflags;
> WaitForSingleObject (p->input_mutex, mutex_timeout);
> GetConsoleMode (p->input_handle, &oflags);
> + DWORD flags = oflags
> + & (ENABLE_EXTENDED_FLAGS | ENABLE_INSERT_MODE |
> ENABLE_QUICK_EDIT_MODE);
> switch (m)
> {
> case tty::restore:
> - flags = ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT |
> ENABLE_PROCESSED_INPUT;
> + flags |= ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT |
> ENABLE_PROCESSED_INPUT;
> break;
> case tty::cygwin:
> - flags = ENABLE_WINDOW_INPUT;
> + flags |= ENABLE_WINDOW_INPUT;
> if (wincap.has_con_24bit_colors () && !con_is_legacy)
> flags |= ENABLE_VIRTUAL_TERMINAL_INPUT;
> else
> --
> 2.35.1
>
>
--
Mitchell Hentges
Engineering Workflow
Mozilla
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Cygwin: console: Maintain EXTENDED_FLAGS state
2022-02-10 15:40 ` Mitchell Hentges
@ 2022-02-11 0:12 ` Takashi Yano
2022-02-11 5:10 ` Mitchell Hentges
0 siblings, 1 reply; 6+ messages in thread
From: Takashi Yano @ 2022-02-11 0:12 UTC (permalink / raw)
To: cygwin-patches
On Thu, 10 Feb 2022 10:40:36 -0500
Mitchell Hentges wrote:
> Thanks, I appreciate it.
> The initial send was via GMail, but I've wired up git-send-email to msmtp,
> and I'm hoping that it's happy now - at least, it looks like tabs are being
> preserved now, which is a good sign.
Pushed along with modifying the commit message.
Thansk!
--
Takashi Yano <takashi.yano@nifty.ne.jp>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Cygwin: console: Maintain EXTENDED_FLAGS state
2022-02-11 0:12 ` Takashi Yano
@ 2022-02-11 5:10 ` Mitchell Hentges
0 siblings, 0 replies; 6+ messages in thread
From: Mitchell Hentges @ 2022-02-11 5:10 UTC (permalink / raw)
To: Takashi Yano; +Cc: cygwin-patches
For future reference, as my git commit message lost some information: the
"[1]" link I was trying to refer to was to here:
https://www.os2museum.com/wp/disabling-quick-edit-mode/
This was my first experience working with email-based patch contributing,
so I apologize for the mis-steps here.
Thanks again :)
On Thu, Feb 10, 2022 at 7:12 PM Takashi Yano <takashi.yano@nifty.ne.jp>
wrote:
> On Thu, 10 Feb 2022 10:40:36 -0500
> Mitchell Hentges wrote:
> > Thanks, I appreciate it.
> > The initial send was via GMail, but I've wired up git-send-email to
> msmtp,
> > and I'm hoping that it's happy now - at least, it looks like tabs are
> being
> > preserved now, which is a good sign.
>
> Pushed along with modifying the commit message.
>
> Thansk!
>
> --
> Takashi Yano <takashi.yano@nifty.ne.jp>
>
--
Mitchell Hentges
Engineering Workflow
Mozilla
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-11 5:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-10 0:09 [PATCH 1/1] Cygwin: console: Maintain EXTENDED_FLAGS state Mitchell Hentges
2022-02-10 8:07 ` Takashi Yano
2022-02-10 15:38 ` Mitchell Hentges
2022-02-10 15:40 ` Mitchell Hentges
2022-02-11 0:12 ` Takashi Yano
2022-02-11 5:10 ` Mitchell Hentges
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).