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

Minor improvements for SplitToOem #14746

Merged
merged 1 commit into from
Feb 2, 2023
Merged

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jan 27, 2023

When working on #14745 I noticed that SplitToOem was in a bit of a poor state
as well. Instead of simply iterating over its deque argument and writing the
results into a new deque it used pop to advance the head of both queues.
This isn't quite exception safe and rather bloaty. Additionally there's no need
to call WideCharToMultiByte twice on each character if we know that the most
verbose encoding is UTF-8 which can't be any more than 4 chars anyways.

Related to #8000.

PR Checklist

  • 2 unit tests cover this ✅

@lhecker lhecker added Product-Conhost For issues in the Console codebase Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. labels Jan 27, 2023
@carlos-zamora
Copy link
Member

@msftbot merge this in 10 minutes

@carlos-zamora carlos-zamora merged commit 8100d24 into main Feb 2, 2023
@carlos-zamora carlos-zamora deleted the dev/lhecker/8000-SplitToOem branch February 2, 2023 21:14
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. Product-Conhost For issues in the Console codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants