Skip to content
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

Only passthrough input changes if the client's in VT input mode #4913

Merged
merged 3 commits into from
Mar 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/host/outputStream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
99 changes: 47 additions & 52 deletions src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/conGetSet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 2 additions & 8 deletions src/terminal/adapter/ut_adapter/adapterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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));
Expand Down