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

Clean up WriteConsoleInputA conversion #15672

Merged

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jul 7, 2023

This commit inlines EventsToUnicode into WriteConsoleInputAImpl
because soon we'll not use deques for events anymore and so the old
code won't work. It cleans up the implementation because I intend to
move all this code directly into InputBuffer to have a better and
tighter control over how text gets converted. UTF-8 input for instance
requires the storage of up to 3 input events and this code is not fit
to handle that. It's also unmaintainable because our input handling
code shouldn't be spread over a dozen files either. 😄

Validation Steps Performed

  • Unit and feature tests are ✅

@lhecker lhecker added the Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. label Jul 7, 2023
@lhecker lhecker changed the title Clean up InputBuffer coalescing and preprocessing Clean up WriteConsoleInputA conversion Jul 7, 2023
Base automatically changed from dev/lhecker/8000-InputBuffer-Write to main July 12, 2023 17:18
@lhecker lhecker force-pushed the dev/lhecker/8000-WriteConsoleInputA branch from 80fcebc to 42ac9b7 Compare July 12, 2023 17:21
Comment on lines +166 to +167
lead.Event.KeyEvent.uChar.AsciiChar,
trail.Event.KeyEvent.uChar.AsciiChar,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey wait a minute, won't this only work for things that take up 2 code units? a UTF-8 string can be up to 4 😉

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it works just as bad as it did before! Well... Sort of. At least it now checks if the trailing event is an actual KEY_EVENT instead of blindly casting it to a KeyEvent struct. 😅

I'm hoping to move this entire block of code into InputBuffer at a later time and come up with a proper caching system.

@lhecker
Copy link
Member Author

lhecker commented Jul 18, 2023

@DHowett If you get a chance would you mind reviewing this again?

@lhecker lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jul 18, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot merged commit 11a9808 into main Jul 18, 2023
15 checks passed
@microsoft-github-policy-service microsoft-github-policy-service bot deleted the dev/lhecker/8000-WriteConsoleInputA branch July 18, 2023 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. AutoMerge Marked for automatic merge by the bot when requirements are met
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants