Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Win32 input mode: fixed some bugs found while testing on real Windows Terminal #1582

Closed
wants to merge 2 commits into from
Closed

Win32 input mode: fixed some bugs found while testing on real Windows Terminal #1582

wants to merge 2 commits into from

Conversation

unxed
Copy link
Contributor

@unxed unxed commented Apr 2, 2023

In particular,

  1. input buffer can contain more than one esc seq
  2. respect buffer size (l variable)
  3. win32 input seq length can be more than 8 bytes
  4. do not mess win32 input with mouse coords events

Original bug report:
https://t.me/far2l_ru/6587

See also:
microsoft/terminal#15083

unxed added 2 commits April 1, 2023 22:56
…enceParser::ParseEscapeSequence.

Fixed them. In particular,

1. input buffer can contain more than one esc seq
2. respect buffer size (l variable)
3. win32 input seq length can be more than 8 bytes
4. do not mess win32 input with mouse coords events
@elfmz
Copy link
Owner

elfmz commented Apr 2, 2023

There re still problems with such approach as it doesnt check sequence to be valid and can accept (and consome as whole) as winterm any multiple sequences that starts by [ and has _ inside.
Also i'm not sure about safety of using sscanf for case where integer parts can be empty or some missing at all.
So i've just rewrote that parsing to IMHO safer manner but cannot test if it still works..

@unxed
Copy link
Contributor Author

unxed commented Apr 2, 2023

The man who suffered from problems in WinTerm tested it, b5f7ba3 works just as well as the one I suggested. So closing, thanks!

@unxed unxed closed this Apr 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants