Skip to content

Remove boolean success return values from TextBuffer #15566

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

Merged
merged 1 commit into from
Jun 22, 2023

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jun 16, 2023

I've removed these because it made some of my new code pretty
convoluted for now good reason as most of these functions aren't
exception safe to begin with. Basically, their boolean status
is often just a pretense because they can crash or throw anyways.

Furthermore, WriteCharsLegacy failed to check the status code
returned by AdjustCursorPosition in some of its parts too.

In the future we should instead probably strive to continue
make our legacy code more exception safe.

@lhecker lhecker added the Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. label Jun 16, 2023
@@ -435,10 +435,9 @@ OutputCellIterator ROW::WriteCells(OutputCellIterator it, const til::CoordType c
return it;
}

bool ROW::SetAttrToEnd(const til::CoordType columnBegin, const TextAttribute attr)
Copy link
Member Author

Choose a reason for hiding this comment

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

Suppressing whitespace changes is required: https://github.com/microsoft/terminal/pull/15566/files?w=1

Practically nothing of substance changed in this PR. 🙂

Comment on lines -2695 to -2701
if (!newBuffer.IncrementCursor())
{
hr = E_OUTOFMEMORY;
break;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

lmao

@carlos-zamora carlos-zamora merged commit f0291c6 into main Jun 22, 2023
@carlos-zamora carlos-zamora deleted the dev/lhecker/8000-bool-cleanup branch June 22, 2023 23:24
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants