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
Refactor: softDispatch
doesn't return null
#181891
Conversation
@@ -953,7 +953,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { | |||
// Respect chords if the allowChords setting is set and it's not Escape. Escape is | |||
// handled specially for Zen Mode's Escape, Escape chord, plus it's important in | |||
// terminals generally | |||
const isValidChord = resolveResult?.kind === ResultKind.MoreChordsNeeded && this._configHelper.config.allowChords && event.key !== 'Escape'; | |||
const isValidChord = resolveResult.kind === ResultKind.MoreChordsNeeded && this._configHelper.config.allowChords && event.key !== 'Escape'; |
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.
Hi @Tyriar
I refactored the keybindings resolver and, consequently, AbstractKeybindingService#softDispatch
a bit in last iteration and propagated the changes to various parts of vscode that use the softDispatch. I introduced a regression in one such place and now trying to make sure my changes in other places make sense.
Would you mind confirming that what this line does matches the desired result? This particular PR doesn't do much, there's a more meaningful diff here - https://github.com/microsoft/vscode/pull/178699/files#
Seems fine, I think the only change that impacts me is that |
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 pushed one more change on top to adopt the change in two more places.
Built on #181887
cc @roblourens @lramos15