On Thu, 18 Dec 2025 10:04:13 +0100 (CET)
Johannes Schindelin wrote:
> Hi Takashi,
>
> On Thu, 18 Dec 2025, Takashi Yano wrote:
>
> > In Windows 11, the command "rlwrap cmd" outputs garbaged screen.
> > This is because rlwrap treats text between NLs as a line, while
> > pseudo console sometimes omits NL before "CSIm;nH". This issue
> > does not happen in Windows 10. This patch fixes the issue by
> > adding CR NL before "CSIm:nH" into the output from the pseudo
> > console if the OS is Windows 11.
> >
> > Reviewed-by: Corinna Vinschen <[email protected]>
> > Signed-off-by: Takashi Yano <[email protected]>
> > ---
> > winsup/cygwin/fhandler/pty.cc | 44 +++++++++++++++++++++++++++
> > winsup/cygwin/local_includes/wincap.h | 2 ++
> > winsup/cygwin/wincap.cc | 11 +++++++
> > 3 files changed, 57 insertions(+)
> >
> > diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
> > index 3b0b4f073..7acedc165 100644
> > --- a/winsup/cygwin/fhandler/pty.cc
> > +++ b/winsup/cygwin/fhandler/pty.cc
> > @@ -2775,6 +2775,50 @@ fhandler_pty_master::pty_master_fwd_thread (const
> > master_fwd_thread_param_t *p)
> > else
> > state = 0;
> >
> > + /* Workaround for rlwrap in Win11. rlwrap treats text between
> > + NLs as a line, however, pseudo console in Win11 somtimes
> > + omits NL before "CSIm;nH". This does not happen in Win10. */
> > + if (wincap.has_pcon_omit_nl_before_cursor_move ())
> > + {
> > + state = 0;
>
> The pattern of the first two `for()` loops in this function is to reset
> both `state` and `start_at` (even if the `/* Remove OSC Ps ; ? BEL/ST */`
> loop seems not to reset either, which might be a bug). Should `start_at`
> be re-set to 0 here, too?
Hmm, state should be initialized for each parser. But, start_at
is set in state machine when state 0->1. So, not necessary to init.
> > + for (DWORD i = 0; i < rlen; i++)
> > + if (state == 0 && outbuf[i] == '\033')
> > + {
> > + start_at = i;
> > + state++;
> > + continue;
> > + }
> > + else if (state == 1 && outbuf[i] == '[')
> > + {
> > + state++;
> > + continue;
> > + }
> > + else if (state == 2
> > + && (isdigit (outbuf[i]) || outbuf[i] == ';'))
> > + continue;
> > + else if (state == 2 && outbuf[i] == 'H')
> > + {
> > + /* Add omitted CR NL before "CSIm;nH". However, when the
> > + cusor is on the bottom-most line, adding NL might cause
> > + unexpected scrolling. To avoid this, add "CSI H" before
> > + CR NL. */
> > + const char *ins = "\033[H\r\n";
> > + const int ins_len = strlen (ins);
> > + if (rlen + ins_len <= NT_MAX_PATH)
>
> How can we avoid this problem when running out of buffer space?
Cannot.
The worst case is:
"\033[H" => "\033[H\r\n\033[H"
3 byte => 5 byte
So, ristricting the size to read from pipe to NT_MAX_PATH * 5/3
can avoid this situation, but usually rlen is much smaller than
NT_MAX_PATH, where this does not happen, I think.
In the first place, if the buffer is filled up by ReadFFile() and
an ESC sequence is cut off in the middle, the current parser cannot
function correctly.
Do you think the fix for these cases is necessary?
BTW, the problem that you pointed out in IRC, i.e. each state machine
has own 'for' loop, which is not efficient, is more important to me.
I'll refactor this and submit as a non-bugfix patch.
Thanks!
--
Takashi Yano <[email protected]>
