Repository: microsoft/terminal (language: cpp)

Issue:
Refactor the VT DSR handler to support both ANSI and private options
# Description of the new feature/enhancement

Our current implementation for the _Device Status Report_ sequence is only setup to accept ANSI status options. But eventually we'll also need a way to support private DEC options. In particularly, I'm hoping to support a couple of DEC macro status reports as part of issue #14205, so I'd like to get the framework in place for that in advance.

# Proposed technical implementation details (optional)

I figured we could follow the exact same pattern that was introduced to support both ANSI and DEC modes in PR #8469.


Discussion / hints:
Sounds good to me! Thanks!

Fix patch:
diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp
index 78a8ff9da07..ac9911f308d 100644
--- a/src/terminal/adapter/DispatchTypes.hpp
+++ b/src/terminal/adapter/DispatchTypes.hpp
@@ -209,7 +209,7 @@ namespace Microsoft::Console::VirtualTerminal
     class FlaggedEnumValue
     {
     public:
-        static constexpr VTInt mask{ WI_StaticAssertSingleBitSet(Flag) };
+        static constexpr VTInt mask{ Flag };
 
         constexpr FlaggedEnumValue(const VTInt value) :
             _value{ value }
@@ -371,10 +371,14 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes
         Max = SaveBackgroundColor
     };
 
-    enum class AnsiStatusType : VTInt
+    using ANSIStandardStatus = FlaggedEnumValue<0x00000000>;
+    using DECPrivateStatus = FlaggedEnumValue<0x01000000>;
+
+    enum class StatusType : VTInt
     {
-        OS_OperatingStatus = 5,
-        CPR_CursorPositionReport = 6,
+        OS_OperatingStatus = ANSIStandardStatus(5),
+        CPR_CursorPositionReport = ANSIStandardStatus(6),
+        ExCPR_ExtendedCursorPositionReport = DECPrivateStatus(6),
     };
 
     using ANSIStandardMode = FlaggedEnumValue<0x00000000>;
diff --git a/src/terminal/adapter/ITermDispatch.hpp b/src/terminal/adapter/ITermDispatch.hpp
index 70c9e1e6f02..f8be856f136 100644
--- a/src/terminal/adapter/ITermDispatch.hpp
+++ b/src/terminal/adapter/ITermDispatch.hpp
@@ -104,7 +104,7 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch
 
     virtual bool ResetMode(const DispatchTypes::ModeParams param) = 0; // DECRST
 
-    virtual bool DeviceStatusReport(const DispatchTypes::AnsiStatusType statusType) = 0; // DSR, DSR-OS, DSR-CPR
+    virtual bool DeviceStatusReport(const DispatchTypes::StatusType statusType) = 0; // DSR, DSR-OS, DSR-CPR
     virtual bool DeviceAttributes() = 0; // DA1
     virtual bool SecondaryDeviceAttributes() = 0; // DA2
     virtual bool TertiaryDeviceAttributes() = 0; // DA3
diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp
index 7e5fb800f8d..e38396b2431 100644
--- a/src/terminal/adapter/adaptDispatch.cpp
+++ b/src/terminal/adapter/adaptDispatch.cpp
@@ -798,20 +798,22 @@ bool AdaptDispatch::SetLineRendition(const LineRendition rendition)
 
 // Routine Description:
 // - DSR - Reports status of a console property back to the STDIN based on the type of status requested.
-//       - This particular routine responds to ANSI status patterns only (CSI # n), not the DEC format (CSI ? # n)
 // Arguments:
-// - statusType - ANSI status type indicating what property we should report back
+// - statusType - status type indicating what property we should report back
 // Return Value:
 // - True if handled successfully. False otherwise.
-bool AdaptDispatch::DeviceStatusReport(const DispatchTypes::AnsiStatusType statusType)
+bool AdaptDispatch::DeviceStatusReport(const DispatchTypes::StatusType statusType)
 {
     switch (statusType)
     {
-    case DispatchTypes::AnsiStatusType::OS_OperatingStatus:
+    case DispatchTypes::StatusType::OS_OperatingStatus:
         _OperatingStatus();
         return true;
-    case DispatchTypes::AnsiStatusType::CPR_CursorPositionReport:
-        _CursorPositionReport();
+    case DispatchTypes::StatusType::CPR_CursorPositionReport:
+        _CursorPositionReport(false);
+        return true;
+    case DispatchTypes::StatusType::ExCPR_ExtendedCursorPositionReport:
+        _CursorPositionReport(true);
         return true;
     default:
         return false;
@@ -923,12 +925,13 @@ void AdaptDispatch::_OperatingStatus() const
 }
 
 // Routine Description:
-// - DSR-CPR - Reports the current cursor position within the viewport back to the input channel
+// - CPR and DECXCPR- Reports the current cursor position within the viewport,
+//   as well as the current page number if this is an extended report.
 // Arguments:
-// - <none>
+// - extendedReport - Set to true if the report should include the page number
 // Return Value:
 // - <none>
-void AdaptDispatch::_CursorPositionReport()
+void AdaptDispatch::_CursorPositionReport(const bool extendedReport)
 {
     const auto viewport = _api.GetViewport();
     const auto& textBuffer = _api.GetTextBuffer();
@@ -951,9 +954,20 @@ void AdaptDispatch::_CursorPositionReport()
     }
 
     // Now send it back into the input channel of the console.
-    // First format the response string.
-    const auto response = wil::str_printf<std::wstring>(L"\x1b[%d;%dR", cursorPosition.Y, cursorPosition.X);
-    _api.ReturnResponse(response);
+    if (extendedReport)
+    {
+        // An extended report should also include the page number, but for now
+        // we hardcode it to 1, since we don't yet support paging (GH#13892).
+        const auto pageNumber = 1;
+        const auto response = wil::str_printf<std::wstring>(L"\x1b[?%d;%d;%dR", cursorPosition.Y, cursorPosition.X, pageNumber);
+        _api.ReturnResponse(response);
+    }
+    else
+    {
+        // The standard report only returns the cursor position.
+        const auto response = wil::str_printf<std::wstring>(L"\x1b[%d;%dR", cursorPosition.Y, cursorPosition.X);
+        _api.ReturnResponse(response);
+    }
 }
 
 // Routine Description:
diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp
index 034730f2eed..e6331cd8679 100644
--- a/src/terminal/adapter/adaptDispatch.hpp
+++ b/src/terminal/adapter/adaptDispatch.hpp
@@ -60,7 +60,7 @@ namespace Microsoft::Console::VirtualTerminal
         bool SetCharacterProtectionAttribute(const VTParameters options) override; // DECSCA
         bool PushGraphicsRendition(const VTParameters options) override; // XTPUSHSGR
         bool PopGraphicsRendition() override; // XTPOPSGR
-        bool DeviceStatusReport(const DispatchTypes::AnsiStatusType statusType) override; // DSR, DSR-OS, DSR-CPR
+        bool DeviceStatusReport(const DispatchTypes::StatusType statusType) override; // DSR, DSR-OS, DSR-CPR
         bool DeviceAttributes() override; // DA1
         bool SecondaryDeviceAttributes() override; // DA2
         bool TertiaryDeviceAttributes() override; // DA3
@@ -195,7 +195,7 @@ namespace Microsoft::Console::VirtualTerminal
         void _DoSetTopBottomScrollingMargins(const VTInt topMargin,
                                              const VTInt bottomMargin);
         void _OperatingStatus() const;
-        void _CursorPositionReport();
+        void _CursorPositionReport(const bool extendedReport);
 
         bool _GetParserMode(const StateMachine::Mode mode) const;
         void _SetParserMode(const StateMachine::Mode mode, const bool enable);
diff --git a/src/terminal/adapter/termDispatch.hpp b/src/terminal/adapter/termDispatch.hpp
index 607a3bc72bb..fa72b8acc60 100644
--- a/src/terminal/adapter/termDispatch.hpp
+++ b/src/terminal/adapter/termDispatch.hpp
@@ -97,7 +97,7 @@ class Microsoft::Console::VirtualTerminal::TermDispatch : public Microsoft::Cons
 
     bool ResetMode(const DispatchTypes::ModeParams /*param*/) override { return false; } // DECRST
 
-    bool DeviceStatusReport(const DispatchTypes::AnsiStatusType /*statusType*/) override { return false; } // DSR, DSR-OS, DSR-CPR
+    bool DeviceStatusReport(const DispatchTypes::StatusType /*statusType*/) override { return false; } // DSR, DSR-OS, DSR-CPR
     bool DeviceAttributes() override { return false; } // DA1
     bool SecondaryDeviceAttributes() override { return false; } // DA2
     bool TertiaryDeviceAttributes() override { return false; } // DA3
diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp
index 41f476ef1e3..74e1381328c 100644
--- a/src/terminal/adapter/ut_adapter/adapterTest.cpp
+++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp
@@ -1358,7 +1358,7 @@ class AdapterTest
 
         Log::Comment(L"Test 1: Verify failure when using bad status.");
         _testGetSet->PrepData();
-        VERIFY_IS_FALSE(_pDispatch->DeviceStatusReport((DispatchTypes::AnsiStatusType)-1));
+        VERIFY_IS_FALSE(_pDispatch->DeviceStatusReport((DispatchTypes::StatusType)-1));
     }
 
     TEST_METHOD(DeviceStatus_OperatingStatusTests)
@@ -1367,7 +1367,7 @@ class AdapterTest
 
         Log::Comment(L"Test 1: Verify good operating condition.");
         _testGetSet->PrepData();
-        VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::AnsiStatusType::OS_OperatingStatus));
+        VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::OS_OperatingStatus));
 
         _testGetSet->ValidateInputEvent(L"\x1b[0n");
     }
@@ -1390,7 +1390,7 @@ class AdapterTest
             coordCursorExpected.X++;
             coordCursorExpected.Y++;
 
-            VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::AnsiStatusType::CPR_CursorPositionReport));
+            VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::CPR_CursorPositionReport));
 
             wchar_t pwszBuffer[50];
 
@@ -1414,7 +1414,7 @@ class AdapterTest
             // Then note that VT is 1,1 based for the top left, so add 1. (The rest of the console uses 0,0 for array index bases.)
             coordCursorExpectedFirst += til::point{ 1, 1 };
 
-            VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::AnsiStatusType::CPR_CursorPositionReport));
+            VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::CPR_CursorPositionReport));
 
             auto cursorPos = _testGetSet->_textBuffer->GetCursor().GetPosition();
             cursorPos.X++;
@@ -1424,7 +1424,7 @@ class AdapterTest
             auto coordCursorExpectedSecond{ coordCursorExpectedFirst };
             coordCursorExpectedSecond += til::point{ 1, 1 };
 
-            VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::AnsiStatusType::CPR_CursorPositionReport));
+            VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::CPR_CursorPositionReport));
 
             wchar_t pwszBuffer[50];
 
@@ -1433,6 +1433,33 @@ class AdapterTest
         }
     }
 
+    TEST_METHOD(DeviceStatus_ExtendedCursorPositionReportTests)
+    {
+        Log::Comment(L"Starting test...");
+
+        Log::Comment(L"Test 1: Verify extended cursor position report.");
+        _testGetSet->PrepData(CursorX::XCENTER, CursorY::YCENTER);
+
+        // start with the cursor position in the buffer.
+        til::point coordCursorExpected{ _testGetSet->_textBuffer->GetCursor().GetPosition() };
+
+        // to get to VT, we have to adjust it to its position relative to the viewport top.
+        coordCursorExpected.Y -= _testGetSet->_viewport.Top;
+
+        // Then note that VT is 1,1 based for the top left, so add 1. (The rest of the console uses 0,0 for array index bases.)
+        coordCursorExpected.X++;
+        coordCursorExpected.Y++;
+
+        // Until we support paging (GH#13892) the reported page number should always be 1.
+        const auto pageExpected = 1;
+
+        VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::ExCPR_ExtendedCursorPositionReport));
+
+        wchar_t pwszBuffer[50];
+        swprintf_s(pwszBuffer, ARRAYSIZE(pwszBuffer), L"\x1b[?%d;%d;%dR", coordCursorExpected.Y, coordCursorExpected.X, pageExpected);
+        _testGetSet->ValidateInputEvent(pwszBuffer);
+    }
+
     TEST_METHOD(DeviceAttributesTests)
     {
         Log::Comment(L"Starting test...");
diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp
index d6227383e53..aa4b5ea0e45 100644
--- a/src/terminal/parser/OutputStateMachineEngine.cpp
+++ b/src/terminal/parser/OutputStateMachineEngine.cpp
@@ -530,7 +530,11 @@ bool OutputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParamete
         TermTelemetry::Instance().Log(TermTelemetry::Codes::SGR);
         break;
     case CsiActionCodes::DSR_DeviceStatusReport:
-        success = _dispatch->DeviceStatusReport(parameters.at(0));
+        success = _dispatch->DeviceStatusReport(DispatchTypes::ANSIStandardStatus(parameters.at(0)));
+        TermTelemetry::Instance().Log(TermTelemetry::Codes::DSR);
+        break;
+    case CsiActionCodes::DSR_PrivateDeviceStatusReport:
+        success = _dispatch->DeviceStatusReport(DispatchTypes::DECPrivateStatus(parameters.at(0)));
         TermTelemetry::Instance().Log(TermTelemetry::Codes::DSR);
         break;
     case CsiActionCodes::DA_DeviceAttributes:
diff --git a/src/terminal/parser/OutputStateMachineEngine.hpp b/src/terminal/parser/OutputStateMachineEngine.hpp
index eb29ca9de5d..94dd1187f56 100644
--- a/src/terminal/parser/OutputStateMachineEngine.hpp
+++ b/src/terminal/parser/OutputStateMachineEngine.hpp
@@ -134,6 +134,7 @@ namespace Microsoft::Console::VirtualTerminal
             DECRST_PrivateModeReset = VTID("?l"),
             SGR_SetGraphicsRendition = VTID("m"),
             DSR_DeviceStatusReport = VTID("n"),
+            DSR_PrivateDeviceStatusReport = VTID("?n"),
             DECSTBM_SetScrollingRegion = VTID("r"),
             ANSISYSSC_CursorSave = VTID("s"), // NOTE: Overlaps with DECLRMM/DECSLRM. Fix when/if implemented.
             DTTERM_WindowManipulation = VTID("t"), // NOTE: Overlaps with DECSLPP. Fix when/if implemented.
diff --git a/src/terminal/parser/ut_parser/OutputEngineTest.cpp b/src/terminal/parser/ut_parser/OutputEngineTest.cpp
index 02040fabf94..12246eb14d1 100644
--- a/src/terminal/parser/ut_parser/OutputEngineTest.cpp
+++ b/src/terminal/parser/ut_parser/OutputEngineTest.cpp
@@ -1017,7 +1017,7 @@ class StatefulDispatch final : public TermDispatch
         _eraseType{ (DispatchTypes::EraseType)-1 },
         _eraseTypes{},
         _setGraphics{ false },
-        _statusReportType{ (DispatchTypes::AnsiStatusType)-1 },
+        _statusReportType{ (DispatchTypes::StatusType)-1 },
         _deviceStatusReport{ false },
         _deviceAttributes{ false },
         _secondaryDeviceAttributes{ false },
@@ -1203,7 +1203,7 @@ class StatefulDispatch final : public TermDispatch
     }
     CATCH_LOG_RETURN_FALSE()
 
-    bool DeviceStatusReport(const DispatchTypes::AnsiStatusType statusType) noexcept override
+    bool DeviceStatusReport(const DispatchTypes::StatusType statusType) noexcept override
     {
         _deviceStatusReport = true;
         _statusReportType = statusType;
@@ -1502,7 +1502,7 @@ class StatefulDispatch final : public TermDispatch
     DispatchTypes::EraseType _eraseType;
     std::vector<DispatchTypes::EraseType> _eraseTypes;
     bool _setGraphics;
-    DispatchTypes::AnsiStatusType _statusReportType;
+    DispatchTypes::StatusType _statusReportType;
     bool _deviceStatusReport;
     bool _deviceAttributes;
     bool _secondaryDeviceAttributes;
@@ -2338,7 +2338,7 @@ class StateMachineExternalTest final
         mach.ProcessCharacter(L'n');
 
         VERIFY_IS_TRUE(pDispatch->_deviceStatusReport);
-        VERIFY_ARE_EQUAL(DispatchTypes::AnsiStatusType::OS_OperatingStatus, pDispatch->_statusReportType);
+        VERIFY_ARE_EQUAL(DispatchTypes::StatusType::OS_OperatingStatus, pDispatch->_statusReportType);
 
         pDispatch->ClearState();
 
@@ -2349,7 +2349,19 @@ class StateMachineExternalTest final
         mach.ProcessCharacter(L'n');
 
         VERIFY_IS_TRUE(pDispatch->_deviceStatusReport);
-        VERIFY_ARE_EQUAL(DispatchTypes::AnsiStatusType::CPR_CursorPositionReport, pDispatch->_statusReportType);
+        VERIFY_ARE_EQUAL(DispatchTypes::StatusType::CPR_CursorPositionReport, pDispatch->_statusReportType);
+
+        pDispatch->ClearState();
+
+        Log::Comment(L"Test 3: Check DECXCPR (extended cursor position report) case ?6. Should succeed.");
+        mach.ProcessCharacter(AsciiChars::ESC);
+        mach.ProcessCharacter(L'[');
+        mach.ProcessCharacter(L'?');
+        mach.ProcessCharacter(L'6');
+        mach.ProcessCharacter(L'n');
+
+        VERIFY_IS_TRUE(pDispatch->_deviceStatusReport);
+        VERIFY_ARE_EQUAL(DispatchTypes::StatusType::ExCPR_ExtendedCursorPositionReport, pDispatch->_statusReportType);
 
         pDispatch->ClearState();
     }


Test patch: