-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[FHL] Make VTApiRoutines, which does VT translation for output #11264
Changes from 30 commits
eebed6a
759a919
9325839
3314e4a
3372462
9277ff3
7549a04
9bc88d3
f048c7d
8981585
338e62c
d23b9b0
4749b68
1151239
b43fa07
08415e7
ea2a551
dd83607
5e740c8
c85b2c9
3383816
7cb22a1
cd58080
379224a
85ae5bf
daa154c
063ad97
7b2e216
f070e89
72e8ecc
df4614e
d6d8521
b348b29
61e8822
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,9 +77,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
// This event is explicitly revoked in the destructor: does not need weak_ref | ||
_connectionOutputEventToken = _connection.TerminalOutput({ this, &ControlCore::_connectionOutputHandler }); | ||
|
||
_terminal->SetWriteInputCallback([this](std::wstring& wstr) { | ||
_sendInputToConnection(wstr); | ||
}); | ||
auto pfnWriteInput = std::bind(&ControlCore::_sendInputToConnection, this, std::placeholders::_1); | ||
_terminal->SetWriteInputCallback(pfnWriteInput); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hear you, but the whole remainder of this file is already doing it this way, so I'd rather stick to the pattern if that's OK. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just don't totally get why this had to change so drastically ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I roll my eyes at both of you. Fine. I'll convert it back. |
||
|
||
// GH#8969: pre-seed working directory to prevent potential races | ||
_terminal->SetWorkingDirectory(_settings->StartingDirectory()); | ||
|
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 may not compile in Release. You are using a preprocessor define to control whether the constant is visible, but a
constexpr if
(which must compile successfully) wherein you are using the constant.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.
Actually, you're gating the presence of the member using the preprocessor as well.
Try a build from the commandline with
/p:WindowsTerminalBranding=Release
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'd say, keep the member and the constant defined and only gate whether the code actually touches them based on
if constexpr