Allow exceptions from ITerminalApi and TerminalDispatch#12432
Merged
DHowett merged 3 commits intomicrosoft:mainfrom Feb 9, 2022
Merged
Allow exceptions from ITerminalApi and TerminalDispatch#12432DHowett merged 3 commits intomicrosoft:mainfrom
DHowett merged 3 commits intomicrosoft:mainfrom
Conversation
lhecker
approved these changes
Feb 8, 2022
DHowett
added
zPreview-Service-Queued-1.13
and removed
zStable-Service-Queued-1.12
labels
zadjii-msft
approved these changes
Feb 8, 2022
zadjii-msft
added
the
AutoMerge
|
Hello @zadjii-msft! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 4 hours 31 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
miniksa
approved these changes
Feb 8, 2022
Member
|
Love it! Thanks |
DHowett
pushed a commit
that referenced
this pull request
Feb 9, 2022
This PR updates the `ITerminalApi` and `TerminalDispatch` classes to allow exceptions to be thrown in case of errors instead of using boolean return values. ## References This brings the Terminal code into alignment with the `AdaptDispatch` and `ConGetSet` changes made in PR #12247. And while this isn't exactly a fix for #12378, it does at least stop the app from crashing now. ## Detailed Description of the Pull Request / Additional comments All the `TerminalDispatch` methods have had their `noexcept` specifiers dropped, and any `try`/`catch` wrapping removed, so exceptions will now fall through to the `StateMachine` class where they should be safely caught and logged. The same goes for the `ITerminalApi` interface and its implementation in the `Terminal` class. And many of the methods in this interface have also had their `bool` return values changed to `void`, since there is usually not a need for error return values now. ## Validation Steps Performed I've manually tested the `OSC 9;9` sequence described in #12378 and confirmed that it no longer crashes. (cherry picked from commit 9c11e02)
|
🎉 Handy links: |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR updates the
ITerminalApiandTerminalDispatchclasses toallow exceptions to be thrown in case of errors instead of using boolean
return values.
References
This brings the Terminal code into alignment with the
AdaptDispatchand
ConGetSetchanges made in PR #12247.And while this isn't exactly a fix for #12378, it does at least stop the
app from crashing now.
PR Checklist
where discussion took place: Empty path to OSC 9;9; crashes Terminal window #12378
Detailed Description of the Pull Request / Additional comments
All the
TerminalDispatchmethods have had theirnoexceptspecifiersdropped, and any
try/catchwrapping removed, so exceptions will nowfall through to the
StateMachineclass where they should be safelycaught and logged.
The same goes for the
ITerminalApiinterface and its implementation inthe
Terminalclass. And many of the methods in this interface havealso had their
boolreturn values changed tovoid, since there isusually not a need for error return values now.
Validation Steps Performed
I've manually tested the
OSC 9;9sequence described in #12378 andconfirmed that it no longer crashes.