Apply audit mode to TerminalConnection/Core/Settings and WinConPTY libraries #4016
Conversation
…structs, propagate changes out.
…eferences, string views, standard types.
…ng string views everywhere, size_ts over smaller sizes in prep for massive buffers, and so on and so on.
…ing used for calling ProcessString on the parser in the tests.
…sts. Fixed some rule of 5s, suppressed others. Suppressed constexpr suggestions. Use narrow_cast where applicable. Consts on unchanged variables. .at() for array accesses, and so on.
… constexpr and gluing everything back together. Otherwise, the usual suspects of array-to-pointer decay and noexcepts and whatnot.
miniksa
added
Product-Conpty
| try | ||
| { | ||
| return _terminalApi.EraseInDisplay(eraseType); | ||
| } | ||
| catch (...) | ||
| { | ||
| LOG_CAUGHT_EXCEPTION(); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Are all these try/catch blocks just a temporary thing? I was rather hoping the long term plan was to use exceptions instead of return values for error reporting (at least outside the legacy APIs). Just being able to rid of the return value checking all over the place would make things so much more readable. And with exceptions you can also more easily differentiate between different failure cases (e.g. an unsupported operation vs an implementation failure).
There was a problem hiding this comment.
Sure, I'd be happy to have it move entirely over to an exception based model. This was just the style at the time. I don't mean to put more work on you, but you seem to relish in it: if you would like to file a task and migrate all of the VT-related things from return-code-based to exception-based, I'd be happy to call it "code health".
There was a problem hiding this comment.
Serves me right for opening my big mouth. :) I didn't really have a specific plan for this, but I'll do some investigating on the weekend and file an issue when I have a better idea of how I think it might be achieved.
ghost
closed this
|
@zadjii-msft, I fixed it. Restored the other branch, reopened and retargeted this, then deleted the other branch again. |
|
🎉 |
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
This is turning on the auditing of these projects (as enabled by the heavier lifting in the other refactor) and then cleaning up the remaining warnings.
Validation Steps Performed