Fixes DECSLRM reset

Because SCOSC and DECSLRM reset have the same sequence, CSI s is now resolved to SCOSC only when DECLRMM is reset, otherwise this sequence resolves to DECSLRM.
contour-terminal · Aug 14, 2023 · 4e07a87 · 4e07a87
1 parent f0b6012
commit 4e07a87
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 16 deletions.
diff --git a/src/vtbackend/Functions.h b/src/vtbackend/Functions.h
@@ -393,7 +393,7 @@ constexpr inline auto DECSCL      = detail::CSI(std::nullopt, 2, 2, '"', 'p', VT
 constexpr inline auto DECSCPP     = detail::CSI(std::nullopt, 0, 1, '$', '|', VTType::VT100, "DECSCPP", "Select 80 or 132 Columns per Page");
 constexpr inline auto DECSNLS     = detail::CSI(std::nullopt, 0, 1, '*', '|', VTType::VT420, "DECSNLS", "Select number of lines per screen.");
 constexpr inline auto DECSCUSR    = detail::CSI(std::nullopt, 0, 1, ' ', 'q', VTType::VT520, "DECSCUSR", "Set Cursor Style");
-constexpr inline auto DECSLRM     = detail::CSI(std::nullopt, 2, 2, std::nullopt, 's', VTType::VT420, "DECSLRM", "Set left/right margin");
+constexpr inline auto DECSLRM     = detail::CSI(std::nullopt, 0, 2, std::nullopt, 's', VTType::VT420, "DECSLRM", "Set left/right margin");
 constexpr inline auto DECSM       = detail::CSI('?', 1, ArgsMax, std::nullopt, 'h', VTType::VT100, "DECSM", "Set DEC-mode");
 constexpr inline auto DECSTBM     = detail::CSI(std::nullopt, 0, 2, std::nullopt, 'r', VTType::VT100, "DECSTBM", "Set top/bottom margin");
 constexpr inline auto DECSTR      = detail::CSI(std::nullopt, 0, 0, '!', 'p', VTType::VT100, "DECSTR", "Soft terminal reset");
@@ -409,7 +409,7 @@ constexpr inline auto ICH         = detail::CSI(std::nullopt, 0, 1, std::nullopt
 constexpr inline auto IL          = detail::CSI(std::nullopt, 0, 1, std::nullopt, 'L', VTType::VT100, "IL",  "Insert lines");
 constexpr inline auto REP         = detail::CSI(std::nullopt, 1, 1, std::nullopt, 'b', VTType::VT100, "REP", "Repeat the preceding graphic character Ps times");
 constexpr inline auto RM          = detail::CSI(std::nullopt, 1, ArgsMax, std::nullopt, 'l', VTType::VT100, "RM",  "Reset mode");
-constexpr inline auto SCOSC       = detail::CSI(std::nullopt, 0, 0, std::nullopt, 's', VTType::VT100, "SCOSC", "Save Cursor");
+constexpr inline auto SCOSC       = detail::CSI(std::nullopt, 0, 0, std::nullopt, 's', VTType::VT100, "SCOSC", "Save Cursor (available only when DECLRMM is disabled)");
 constexpr inline auto SD          = detail::CSI(std::nullopt, 0, 1, std::nullopt, 'T', VTType::VT100, "SD",  "Scroll down (pan up)");
 constexpr inline auto SETMARK     = detail::CSI('>', 0, 0, std::nullopt, 'M', VTExtension::Contour, "XTSETMARK", "Set Vertical Mark (experimental syntax)");
 constexpr inline auto SGR         = detail::CSI(std::nullopt, 0, ArgsMax, std::nullopt, 'm', VTType::VT100, "SGR", "Select graphics rendition");

diff --git a/src/vtbackend/Screen.cpp b/src/vtbackend/Screen.cpp
@@ -3540,11 +3540,19 @@ ApplyResult Screen<Cell>::apply(FunctionDefinition const& function, Sequence con
         case DECSNLS:
             _terminal.resizeScreen(PageSize { pageSize().lines, seq.param<ColumnCount>(0) });
             return ApplyResult::Ok;
+        case SCOSC: [[fallthrough]]; // Only works when DECLRMM is disabled (Xterm Compatibility)
         case DECSLRM: {
             auto l = decr(seq.param_opt<ColumnOffset>(0));
             auto r = decr(seq.param_opt<ColumnOffset>(1));
-            _terminal.setLeftRightMargin(l, r);
-            moveCursorTo({}, {});
+            if (_terminal.isModeEnabled(DECMode::LeftRightMargin))
+            {
+                _terminal.setLeftRightMargin(l, r);
+                moveCursorTo({}, {});
+            }
+            else
+            {
+                saveCursor();
+            }
         }
         break;
         case DECSM: {
@@ -3617,7 +3625,6 @@ ApplyResult Screen<Cell>::apply(FunctionDefinition const& function, Sequence con
             return r;
         }
         break;
-        case SCOSC: saveCursor(); break;
         case SD: scrollDown(seq.param_or<LineCount>(0, LineCount { 1 })); break;
         case SETMARK: setMark(); break;
         case SGR: return impl::applySGR(_terminal, seq, 0, seq.parameterCount());

diff --git a/src/vtbackend/Screen_test.cpp b/src/vtbackend/Screen_test.cpp
@@ -1581,7 +1581,10 @@ TEST_CASE("ScrollUp.WithMargins", "[screen]")
                 == screen.renderMainPageText());
     }
     mock.writeToScreen("\033[r");
+    mock.writeToScreen("\033[s");
     REQUIRE(screen.margin().vertical == Margin::Vertical { .from = LineOffset(0), .to = LineOffset(4) });
+    REQUIRE(screen.margin().horizontal
+            == Margin::Horizontal { .from = ColumnOffset(0), .to = ColumnOffset(4) });
 }
 
 TEST_CASE("ScrollUp", "[screen]")

diff --git a/src/vtbackend/Terminal.cpp b/src/vtbackend/Terminal.cpp
@@ -1654,17 +1654,14 @@ void Terminal::setTopBottomMargin(optional<LineOffset> top, optional<LineOffset>
 
 void Terminal::setLeftRightMargin(optional<ColumnOffset> left, optional<ColumnOffset> right)
 {
-    if (isModeEnabled(DECMode::LeftRightMargin))
-    {
-        auto const defaultLeft = ColumnOffset(0);
-        auto const defaultRight = boxed_cast<ColumnOffset>(_settings.pageSize.columns) - 1;
-        auto const sanitizedRight = std::min(right.value_or(defaultRight), defaultRight);
-        auto const sanitizedLeft = std::max(left.value_or(defaultLeft), defaultLeft);
-        if (left < right)
-        {
-            currentScreen().margin().horizontal.from = sanitizedLeft;
-            currentScreen().margin().horizontal.to = sanitizedRight;
-        }
+    auto const defaultLeft = ColumnOffset(0);
+    auto const defaultRight = boxed_cast<ColumnOffset>(_settings.pageSize.columns) - 1;
+    auto const sanitizedRight = std::min(right.value_or(defaultRight), defaultRight);
+    auto const sanitizedLeft = std::max(left.value_or(defaultLeft), defaultLeft);
+    if (sanitizedLeft < sanitizedRight)
+    {
+        currentScreen().margin().horizontal.from = sanitizedLeft;
+        currentScreen().margin().horizontal.to = sanitizedRight;
     }
 }