-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
Use modern loops #5152
Use modern loops #5152
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is neat, thanks!
@@ -80,9 +80,8 @@ void TextAttributeTests::TestRoundtripMetaBits() | |||
COMMON_LVB_UNDERSCORE | |||
}; | |||
|
|||
for (int i = 0; i < ARRAYSIZE(metaFlags); ++i) | |||
for (unsigned short flag : metaFlags) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had no idea that this just worked in c++, that's really cool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does except the main problem is for some reason in the tracelogging .cpp, I had to unroll the changes because of what I think is a compiler bug because only in that file would the compiler emit an error.
@@ -1055,14 +1055,14 @@ void InputEngineTest::VerifySGRMouseData(const std::vector<std::tuple<SGR_PARAMS | |||
SGR_PARAMS input; | |||
MOUSE_EVENT_PARAMS expected; | |||
INPUT_RECORD inputRec; | |||
for (size_t i = 0; i < testData.size(); i++) | |||
for (const auto& i : testData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this not be for (const auto& [sgrParam, mouseEventParam]) : testData
to split the tuple up all at once and skip the std::get
?
// Return value: | ||
// - none | ||
// Note: may throw exception on allocation error | ||
void SCREEN_INFORMATION::AddTabStop(const SHORT sColumn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there may have been a merge conflict -- this reintroduces the tab stop functions that were deleted in master
Summary of the Pull Request
I converted many of the older loops to the modern one, which is much shorter and is easier to understand.
References
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed