diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index 55f0cfcd693..dfac1d27f46 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -769,14 +769,17 @@ bool ConhostInternalGetSet::SetCursorColor(const COLORREF cursorColor) // Routine Description: // - Connects the IsConsolePty call directly into our Driver Message servicing call inside Conhost.exe +// - NOTE: This ONE method behaves differently! The rest of the methods on this +// interface return true if successful. This one just returns the result. // Arguments: // - isPty: receives the bool indicating whether or not we're in pty mode. // Return Value: -// - true if successful (see DoSrvIsConsolePty). false otherwise. -bool ConhostInternalGetSet::IsConsolePty(bool& isPty) const +// - true if we're in pty mode. +bool ConhostInternalGetSet::IsConsolePty() const { + bool isPty = false; DoSrvIsConsolePty(isPty); - return true; + return isPty; } bool ConhostInternalGetSet::DeleteLines(const size_t count) diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index eff56c90be2..7544041213a 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -143,7 +143,7 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: bool GetConsoleOutputCP(unsigned int& codepage) override; - bool IsConsolePty(bool& isPty) const override; + bool IsConsolePty() const override; bool DeleteLines(const size_t count) override; bool InsertLines(const size_t count) override; diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 35eb0b64eac..091d0b892c0 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -583,8 +583,7 @@ bool AdaptDispatch::EraseInDisplay(const DispatchTypes::EraseType eraseType) // make the state machine propogate this ED sequence to the connected // terminal application. While we're in conpty mode, we don't really // have a scrollback, but the attached terminal might. - bool isPty = false; - _pConApi->IsConsolePty(isPty); + const bool isPty = _pConApi->IsConsolePty(); return eraseScrollbackResult && (!isPty); } else if (eraseType == DispatchTypes::EraseType::All) @@ -1042,10 +1041,11 @@ bool AdaptDispatch::SetKeypadMode(const bool fApplicationMode) bool success = true; success = _pConApi->PrivateSetKeypadMode(fApplicationMode); - // If we're a conpty, always return false - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + // If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false + // The VT Input mode check is to work around ssh.exe v7.7, which uses VT + // output, but not Input. Once the conpty supports these types of input, + // this check can be removed. See GH#4911 + if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled()) { return false; } @@ -1063,10 +1063,11 @@ bool AdaptDispatch::SetCursorKeysMode(const bool applicationMode) bool success = true; success = _pConApi->PrivateSetCursorKeysMode(applicationMode); - // If we're a conpty, always return false - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + // If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false + // The VT Input mode check is to work around ssh.exe v7.7, which uses VT + // output, but not Input. Once the conpty supports these types of input, + // this check can be removed. See GH#4911 + if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled()) { return false; } @@ -1558,9 +1559,7 @@ bool AdaptDispatch::HardReset() // make the state machine propogate this RIS sequence to the connected // terminal application. We've reset our state, but the connected terminal // might need to do more. - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + if (_pConApi->IsConsolePty()) { return false; } @@ -1713,10 +1712,11 @@ bool AdaptDispatch::EnableVT200MouseMode(const bool enabled) bool success = true; success = _pConApi->PrivateEnableVT200MouseMode(enabled); - // If we're a conpty, always return false - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + // If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false + // The VT Input mode check is to work around ssh.exe v7.7, which uses VT + // output, but not Input. Once the conpty supports these types of input, + // this check can be removed. See GH#4911 + if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled()) { return false; } @@ -1736,10 +1736,11 @@ bool AdaptDispatch::EnableUTF8ExtendedMouseMode(const bool enabled) bool success = true; success = _pConApi->PrivateEnableUTF8ExtendedMouseMode(enabled); - // If we're a conpty, always return false - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + // If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false + // The VT Input mode check is to work around ssh.exe v7.7, which uses VT + // output, but not Input. Once the conpty supports these types of input, + // this check can be removed. See GH#4911 + if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled()) { return false; } @@ -1759,10 +1760,11 @@ bool AdaptDispatch::EnableSGRExtendedMouseMode(const bool enabled) bool success = true; success = _pConApi->PrivateEnableSGRExtendedMouseMode(enabled); - // If we're a conpty, always return false - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + // If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false + // The VT Input mode check is to work around ssh.exe v7.7, which uses VT + // output, but not Input. Once the conpty supports these types of input, + // this check can be removed. See GH#4911 + if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled()) { return false; } @@ -1781,10 +1783,11 @@ bool AdaptDispatch::EnableButtonEventMouseMode(const bool enabled) bool success = true; success = _pConApi->PrivateEnableButtonEventMouseMode(enabled); - // If we're a conpty, always return false - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + // If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false + // The VT Input mode check is to work around ssh.exe v7.7, which uses VT + // output, but not Input. Once the conpty supports these types of input, + // this check can be removed. See GH#4911 + if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled()) { return false; } @@ -1804,10 +1807,11 @@ bool AdaptDispatch::EnableAnyEventMouseMode(const bool enabled) bool success = true; success = _pConApi->PrivateEnableAnyEventMouseMode(enabled); - // If we're a conpty, always return false - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + // If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false + // The VT Input mode check is to work around ssh.exe v7.7, which uses VT + // output, but not Input. Once the conpty supports these types of input, + // this check can be removed. See GH#4911 + if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled()) { return false; } @@ -1827,10 +1831,11 @@ bool AdaptDispatch::EnableAlternateScroll(const bool enabled) bool success = true; success = _pConApi->PrivateEnableAlternateScroll(enabled); - // If we're a conpty, always return false - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + // If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false + // The VT Input mode check is to work around ssh.exe v7.7, which uses VT + // output, but not Input. Once the conpty supports these types of input, + // this check can be removed. See GH#4911 + if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled()) { return false; } @@ -1889,9 +1894,7 @@ bool AdaptDispatch::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle) // If we're a conpty, always return false, so that this cursor state will be // sent to the connected terminal - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + if (_pConApi->IsConsolePty()) { return false; } @@ -1908,9 +1911,7 @@ bool AdaptDispatch::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle) // True if handled successfully. False otherwise. bool AdaptDispatch::SetCursorColor(const COLORREF cursorColor) { - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + if (_pConApi->IsConsolePty()) { return false; } @@ -1938,9 +1939,7 @@ bool AdaptDispatch::SetColorTableEntry(const size_t tableIndex, const DWORD dwCo // value to the terminal. Still handle the sequence so apps that use // the API or VT to query the values of the color table still read the // correct color. - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + if (_pConApi->IsConsolePty()) { return false; } @@ -1963,9 +1962,7 @@ bool Microsoft::Console::VirtualTerminal::AdaptDispatch::SetDefaultForeground(co // value to the terminal. Still handle the sequence so apps that use // the API or VT to query the values of the color table still read the // correct color. - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + if (_pConApi->IsConsolePty()) { return false; } @@ -1988,9 +1985,7 @@ bool Microsoft::Console::VirtualTerminal::AdaptDispatch::SetDefaultBackground(co // value to the terminal. Still handle the sequence so apps that use // the API or VT to query the values of the color table still read the // correct color. - bool isPty = false; - _pConApi->IsConsolePty(isPty); - if (isPty) + if (_pConApi->IsConsolePty()) { return false; } diff --git a/src/terminal/adapter/conGetSet.hpp b/src/terminal/adapter/conGetSet.hpp index 8a41cf1fa71..6516db3881a 100644 --- a/src/terminal/adapter/conGetSet.hpp +++ b/src/terminal/adapter/conGetSet.hpp @@ -98,7 +98,7 @@ namespace Microsoft::Console::VirtualTerminal virtual bool GetConsoleOutputCP(unsigned int& codepage) = 0; virtual bool PrivateSuppressResizeRepaint() = 0; - virtual bool IsConsolePty(bool& isPty) const = 0; + virtual bool IsConsolePty() const = 0; virtual bool DeleteLines(const size_t count) = 0; virtual bool InsertLines(const size_t count) = 0; diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index e98e69991ec..241d9eb7e74 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -610,14 +610,10 @@ class TestGetSet final : public ConGetSet return _getConsoleOutputCPResult; } - bool IsConsolePty(bool& isPty) const override + bool IsConsolePty() const override { Log::Comment(L"IsConsolePty MOCK called..."); - if (_isConsolePtyResult) - { - isPty = _isPty; - } - return _isConsolePtyResult; + return _isPty; } bool DeleteLines(const size_t /*count*/) override @@ -951,7 +947,6 @@ class TestGetSet final : public ConGetSet bool _setCursorColorResult = false; COLORREF _expectedCursorColor = 0; bool _getConsoleOutputCPResult = false; - bool _isConsolePtyResult = false; bool _privateSetDefaultAttributesResult = false; bool _moveToBottomResult = false; @@ -2270,7 +2265,6 @@ class AdapterTest // Test in pty mode - we should fail, but PrivateSetColorTableEntry should still be called _testGetSet->_isPty = true; - _testGetSet->_isConsolePtyResult = true; _testGetSet->_expectedColorTableIndex = 15; // Windows BRIGHT_WHITE VERIFY_IS_FALSE(_pDispatch.get()->SetColorTableEntry(15, testColor));