Repository: microsoft/terminal (language: cpp)

Issue:
Support colons as a VT parameter separator
<!-- 
🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨

I ACKNOWLEDGE THE FOLLOWING BEFORE PROCEEDING:
1. If I delete this entire template and go my own path, the core team may close my issue without further explanation or engagement.
2. If I list multiple bugs/concerns in this one issue, the core team may close my issue without further explanation or engagement.
3. If I write an issue that has many duplicates, the core team may close my issue without further explanation or engagement (and without necessarily spending time to find the exact duplicate ID number).
4. If I leave the title incomplete when filing the issue, the core team may close my issue without further explanation or engagement.
5. If I file something completely blank in the body, the core team may close my issue without further explanation or engagement.

All good? Then proceed!
-->

<!--
This bug tracker is monitored by Windows Terminal development team and other technical folks.

**Important: When reporting BSODs or security issues, DO NOT attach memory dumps, logs, or traces to Github issues**.
Instead, send dumps/traces to secure@microsoft.com, referencing this GitHub issue.

If this is an application crash, please also provide a Feedback Hub submission link so we can find your diagnostic data on the backend. Use the category "Apps > Windows Terminal (Preview)" and choose "Share My Feedback" after submission to get the link.

Please use this form and describe your issue, concisely but precisely, with as much detail as possible.

-->

# Environment

```none
Windows build number: Microsoft Windows [Version 10.0.19041.21]
Windows Terminal version (if applicable): 0.8.10091.0

Any other software?  WSL, Ubuntu
```

# Steps to reproduce

I have TERM set to xterm-24bit using the standard termconfig instructions that are online.  Running emacs in terminal mode (emacs -nw) and then using M-x list-display-colors


# Behavior (windows terminal on the left, wsltty on the right)

![image](https://user-images.githubusercontent.com/26091250/72826584-1d4ab000-3c47-11ea-95ea-369fc31edd49.png)

In both cases, emacs knows about all 500+ colors available, but wsltty shows them and windows terminal doesn't.  Also this has been like this for a couple months at least, but now it's bugging me enough to ask for a fix. :)


Discussion / hints:
Does this repro in a legacy console window? (i.e. not wsltty, but `conhost.exe` running `wsl` directly)?

Could you link to the 

> standard termconfig instructions

for `xterm-24bit`?
Yeah, the terminfo definition is likely to be the issue here. I'm guessing it's using the colon separator syntax for 24-bit colors which mintty supports but we don't. If that's the case you can just replaces the colons with semicolons in the terminfo file and it should be fine.
Actually, I've just found the instructions in the Emacs FAQ for setting up the 24bit terminfo (see [here](https://www.gnu.org/software/emacs/manual/html_node/efaq/Colors-on-a-TTY.html)). They create two terminfo entries: one using the colon syntax, and one with the semicolon syntax. If those are the instructions you followed, then you can just set your TERM to `xterm-24bits` (with an `s`) and it should use the semicolon syntax.
Oh geez.  You are correct.  I switched to my already defined xterm-24bits and that worked.

So yeah, this is not a bug.  Maybe a request to indicate if terminfo isn't being parsed correctly due to colons?

I'm not sure we can generally know that. Alas!
It's probably worth adding support for the ITU colon syntax to the backlog, though. It may not be as widely used as the semicolon syntax, but it's still worth supporting if possible.
> It's probably worth adding support for the ITU colon syntax to the backlog, though. It may not be as widely used as the semicolon syntax, but it's still worth supporting if possible.

Let's re-purpose this issue as that thread then 😄 

Relevant:
https://gitlab.com/gnachman/iterm2/issues/6377



See XVilka's gist, in particular:
- [This comment](https://gist.github.com/XVilka/8346728#gistcomment-2008553) about why it's important to get the parser right, and why lazy programmer approaches (e.g. treating `:` as a synonym for `;`) are truly harmful;
- [A few comments starting here](https://gist.github.com/XVilka/8346728#gistcomment-2774523) (Dec 2-5, 2018) how the spec is unclear about the _first_ separator being `:` or `;` (note though that implementations tend to go with `:` as that clearly makes more sense);

and of course the spec ITU T.416 itself, too.

[On a side note... Even after years of developing a terminal, I still keep getting uncertain whether nowadays' most popular terminal description is called `xterm-256color` or `xterm-256colors`. Emacs's decision of introducing something that is seemingly a singular vs. plural difference to denote the separator (that final `s` in `xterm-24bits` presumably standing for `semicolon`) was... let's just say probably not a great idea :)]

Pulling the triage tag on this. This seems totally reasonable, and I appreciate your insight on how we shouldn't just treat `:` and `;` as equivalent, @egmontkob :smile:

Thanks!
I'm working on this. First step is #7220, and the additional delta over switching the VT engine to it is +/- 26/2 lines (not accounting for tests). :smile:
Since I don't see it mentioned anywhere here, [ansi-vte52.sh](https://github.com/csdvrx/sixel-testsuite/blob/31596c68ff960fd45457a6f0d8d86fa5614bc502/ansi-vte52.sh) briefly demonstrates the ITU-T T.416 forms of SGR 38 and SGR 48, including an incorrect form of the truecolour specification (where the colourspace ID was missed) [apparently originating from the KDE konsole application](https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h4-Functions-using-CSI-_-ordered-by-the-final-character-lparen-s-rparen:CSI-Pm-m:Ps-=-3-8;2;Pr;Pg;Pb.21AA), although ["pretty much everyone" independently made the same mistake](https://gitlab.com/gnachman/iterm2/-/issues/6377).
This is an important feature, hopefully it can get some attention.  As long as popular terminals don't support the correct syntax it incentivizes everyone to continue building with and relying on the broken syntax.

To be honest, it probably should have gone in before 1.0 was declared to minimize the damage.  Windows has an oversized influence and so a greater responsibility, just like Spider-man.  ;-)
I'd love to see this happen soon-ish. I have a bunch of in-house tools that have to support both fancy (256 / truecolor) and limited (16-color only) terminals, and the lack of support for the `:` separator means Windows Terminal doesn't get full colors.

I can safely use the `\e[...;38:2:R:G:B;...m` form and the truecolor part will be ignored as a single unknown control sequence by the old 16-color terminal. But if I were to use `;` as a separator, it will instead interpret it as 5 _separate_ control sequences, and then ignore those that it doesn't recognize and interpret the others, and then I end up e.g. with blinking inverted text and stuff like that.

I can't just detect the terminal type, as large parts of the output are static text files that have been pre-generated. (And I can't justify re-architecting that just for Windows Terminal.) So I'm stuck with only using `:` as a separator, and effectively only getting 16 colors on Windows.
> I can safely use the `\e[...;38:2:R:G:B;...m` form and the truecolor part will be ignored as a single unknown control sequence by the old 16-color terminal.

@229c9cf0 Just FYI, there are terminals out there that will simply echo the sequence parameters to the screen when they encounter a colon in an escape sequence, so it's not really that safe. Also, the "correct" syntax for the colon format is actually `38:2::R:G:B`. Some terminals support both colon variants, some only support one or the other, but everyone seems to support the semicolon syntax (assuming they support true color at all).

> And I can't justify re-architecting that just for Windows Terminal

I don't know what your target audience is, but in case you aren't aware, Windows Terminal is by no means the only terminal that doesn't support the colon syntax. You can get a general idea of the level of support at this website: https://gist.github.com/XVilka/8346728. But note that it's not always correct, so it's best to confirm for yourself. For example, I'm pretty sure that Konsole *doesn't* support colons, while more recent versions of Alacritty actually *do* support colons now.

> You can get a general idea of the level of support at this website: https://gist.github.com/XVilka/8346728. But note that it's not always correct, so it's best to confirm for yourself.

Updates for that Gist now go to https://github.com/termstandard/colors, and they also take PRs to fix out of date or wrong info.
Perhaps this could be implemented by adding a `bool isSubparameter` member to `VTParameter`, populating that in `StateMachine::_ActionParam(const wchar_t wch)`, and making `AdaptDispatch::_ApplyGraphicsOptions` skip any subparameters not handled by `AdaptDispatch::_ApplyGraphicsOption`.

Fix patch:
diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp
index 9e342ec5f80..81864c0c503 100644
--- a/src/terminal/adapter/DispatchTypes.hpp
+++ b/src/terminal/adapter/DispatchTypes.hpp
@@ -152,6 +152,48 @@ namespace Microsoft::Console::VirtualTerminal
         VTInt _value;
     };
 
+    class VTSubParameters
+    {
+    public:
+        constexpr VTSubParameters() noexcept
+        {
+        }
+
+        constexpr VTSubParameters(const std::span<const VTParameter> subParams) noexcept :
+            _subParams{ subParams }
+        {
+        }
+
+        constexpr VTParameter at(const size_t index) const noexcept
+        {
+            return til::at(_subParams, index);
+        }
+
+        VTSubParameters subspan(const size_t offset, const size_t count) const noexcept
+        {
+            const auto subParamsSpan = _subParams.subspan(offset, count);
+            return { subParamsSpan };
+        }
+
+        bool empty() const noexcept
+        {
+            return _subParams.empty();
+        }
+
+        size_t size() const noexcept
+        {
+            return _subParams.size();
+        }
+
+        constexpr operator std::span<const VTParameter>() const noexcept
+        {
+            return _subParams;
+        }
+
+    private:
+        std::span<const VTParameter> _subParams;
+    };
+
     class VTParameters
     {
     public:
@@ -159,49 +201,95 @@ namespace Microsoft::Console::VirtualTerminal
         {
         }
 
-        constexpr VTParameters(const VTParameter* ptr, const size_t count) noexcept :
-            _values{ ptr, count }
+        constexpr VTParameters(const VTParameter* paramsPtr, const size_t paramsCount) noexcept :
+            _params{ paramsPtr, paramsCount },
+            _subParams{},
+            _subParamRanges{}
+        {
+        }
+
+        constexpr VTParameters(const std::span<const VTParameter> params,
+                               const std::span<const VTParameter> subParams,
+                               const std::span<const std::pair<BYTE, BYTE>> subParamRanges) noexcept :
+            _params{ params },
+            _subParams{ subParams },
+            _subParamRanges{ subParamRanges }
         {
         }
 
         constexpr VTParameter at(const size_t index) const noexcept
         {
             // If the index is out of range, we return a parameter with no value.
-            return index < _values.size() ? til::at(_values, index) : defaultParameter;
+            return index < _params.size() ? til::at(_params, index) : defaultParameter;
         }
 
         constexpr bool empty() const noexcept
         {
-            return _values.empty();
+            return _params.empty();
         }
 
         constexpr size_t size() const noexcept
         {
             // We always return a size of at least 1, since an empty parameter
             // list is the equivalent of a single "default" parameter.
-            return std::max<size_t>(_values.size(), 1);
+            return std::max<size_t>(_params.size(), 1);
         }
 
         VTParameters subspan(const size_t offset) const noexcept
         {
-            const auto subValues = _values.subspan(std::min(offset, _values.size()));
-            return { subValues.data(), subValues.size() };
+            // We need sub parameters to always be in their original index
+            // because we store their indexes in subParamRanges. So we pass
+            // _subParams as is and create new span for others.
+            const auto newParamsSpan = _params.subspan(std::min(offset, _params.size()));
+            const auto newSubParamRangesSpan = _subParamRanges.subspan(std::min(offset, _subParamRanges.size()));
+            return { newParamsSpan, _subParams, newSubParamRangesSpan };
+        }
+
+        VTSubParameters subParamsFor(const size_t index) const noexcept
+        {
+            if (index < _subParamRanges.size())
+            {
+                const auto& range = til::at(_subParamRanges, index);
+                return _subParams.subspan(range.first, range.second - range.first);
+            }
+            else
+            {
+                return VTSubParameters{};
+            }
+        }
+
+        bool hasSubParams() const noexcept
+        {
+            return !_subParams.empty();
+        }
+
+        bool hasSubParamsFor(const size_t index) const noexcept
+        {
+            if (index < _subParamRanges.size())
+            {
+                const auto& range = til::at(_subParamRanges, index);
+                return range.second > range.first;
+            }
+            else
+            {
+                return false;
+            }
         }
 
         template<typename T>
         bool for_each(const T&& predicate) const
         {
-            auto values = _values;
+            auto params = _params;
 
             // We always return at least 1 value here, since an empty parameter
             // list is the equivalent of a single "default" parameter.
-            if (values.empty())
+            if (params.empty())
             {
-                values = defaultParameters;
+                params = defaultParameters;
             }
 
             auto success = true;
-            for (const auto& v : values)
+            for (const auto& v : params)
             {
                 success = predicate(v) && success;
             }
@@ -209,10 +297,12 @@ namespace Microsoft::Console::VirtualTerminal
         }
 
     private:
-        static constexpr VTParameter defaultParameter;
+        static constexpr VTParameter defaultParameter{};
         static constexpr std::span defaultParameters{ &defaultParameter, 1 };
 
-        std::span<const VTParameter> _values;
+        std::span<const VTParameter> _params;
+        VTSubParameters _subParams;
+        std::span<const std::pair<BYTE, BYTE>> _subParamRanges;
     };
 
     // FlaggedEnumValue is a convenience class that produces enum values (of a specified size)
diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp
index 674cafc4506..e0c346dc9d0 100644
--- a/src/terminal/adapter/adaptDispatch.hpp
+++ b/src/terminal/adapter/adaptDispatch.hpp
@@ -300,6 +300,9 @@ namespace Microsoft::Console::VirtualTerminal
         size_t _ApplyGraphicsOption(const VTParameters options,
                                     const size_t optionIndex,
                                     TextAttribute& attr) noexcept;
+        void _ApplyGraphicsOptionSubParam(const VTParameter option,
+                                          const VTSubParameters subParams,
+                                          TextAttribute& attr) noexcept;
         void _ApplyGraphicsOptions(const VTParameters options,
                                    TextAttribute& attr) noexcept;
 
diff --git a/src/terminal/adapter/adaptDispatchGraphics.cpp b/src/terminal/adapter/adaptDispatchGraphics.cpp
index 0de71a293ae..5259cf8ce64 100644
--- a/src/terminal/adapter/adaptDispatchGraphics.cpp
+++ b/src/terminal/adapter/adaptDispatchGraphics.cpp
@@ -64,6 +64,7 @@ size_t AdaptDispatch::_SetRgbColorsHelper(const VTParameters options,
 
 // Routine Description:
 // - Helper to apply a single graphic rendition option to an attribute.
+// - Calls appropriate helper to apply the option with sub parameters when necessary.
 // Arguments:
 // - options - An array of options.
 // - optionIndex - The start index of the option that will be applied.
@@ -75,6 +76,14 @@ size_t AdaptDispatch::_ApplyGraphicsOption(const VTParameters options,
                                            TextAttribute& attr) noexcept
 {
     const GraphicsOptions opt = options.at(optionIndex);
+
+    if (options.hasSubParamsFor(optionIndex))
+    {
+        const auto subParams = options.subParamsFor(optionIndex);
+        _ApplyGraphicsOptionSubParam(opt, subParams, attr);
+        return 1;
+    }
+
     switch (opt)
     {
     case Off:
@@ -250,6 +259,23 @@ size_t AdaptDispatch::_ApplyGraphicsOption(const VTParameters options,
     }
 }
 
+// Routine Description:
+// - This is a no-op until we have something meaningful to do with sub parameters.
+// - Helper to apply a single graphic rendition option with sub parameters to an attribute.
+// Arguments:
+// - option - An option to apply.
+// - attr - The attribute that will be updated with the applied option.
+// Return Value:
+// - <None>
+void AdaptDispatch::_ApplyGraphicsOptionSubParam(const VTParameter /* option */,
+                                                 const VTSubParameters /* subParam */,
+                                                 TextAttribute& /* attr */) noexcept
+{
+    // here, we apply our "best effort" rule, while handling sub params if we don't
+    // recognise the parameter substring (parameter and it's sub parameters) then
+    // we should just skip over them.
+}
+
 // Routine Description:
 // - Helper to apply a number of graphic rendition options to an attribute.
 // Arguments:
diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp
index 84ec0fa90e0..80a77d89173 100644
--- a/src/terminal/parser/OutputStateMachineEngine.cpp
+++ b/src/terminal/parser/OutputStateMachineEngine.cpp
@@ -416,6 +416,12 @@ bool OutputStateMachineEngine::ActionVt52EscDispatch(const VTID id, const VTPara
 // - true iff we successfully dispatched the sequence.
 bool OutputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParameters parameters)
 {
+    // Bail out if we receive subparameters, but we don't accept them in the sequence.
+    if (parameters.hasSubParams() && !_CanSeqAcceptSubParam(id, parameters)) [[unlikely]]
+    {
+        return false;
+    }
+
     auto success = false;
 
     switch (id)
@@ -1098,6 +1104,26 @@ bool OutputStateMachineEngine::_GetOscSetClipboard(const std::wstring_view strin
     return SUCCEEDED_LOG(Base64::Decode(substr, content));
 }
 
+// Routine Description:
+// - Takes a sequence id ("final byte") and determines if it accepts sub parameters.
+// Arguments:
+// - id - The sequence id to check for.
+// Return Value:
+// - True, if it accepts sub parameters or else False.
+bool OutputStateMachineEngine::_CanSeqAcceptSubParam(const VTID id, const VTParameters& parameters) noexcept
+{
+    switch (id)
+    {
+    case SGR_SetGraphicsRendition:
+        return true;
+    case DECCARA_ChangeAttributesRectangularArea:
+    case DECRARA_ReverseAttributesRectangularArea:
+        return !parameters.hasSubParamsFor(0) && !parameters.hasSubParamsFor(1) && !parameters.hasSubParamsFor(2) && !parameters.hasSubParamsFor(3);
+    default:
+        return false;
+    }
+}
+
 // Method Description:
 // - Clears our last stored character. The last stored character is the last
 //      graphical character we printed, which is reset if any other action is
diff --git a/src/terminal/parser/OutputStateMachineEngine.hpp b/src/terminal/parser/OutputStateMachineEngine.hpp
index edb8982f7da..b078ade9fb7 100644
--- a/src/terminal/parser/OutputStateMachineEngine.hpp
+++ b/src/terminal/parser/OutputStateMachineEngine.hpp
@@ -236,6 +236,8 @@ namespace Microsoft::Console::VirtualTerminal
                              std::wstring& params,
                              std::wstring& uri) const;
 
+        bool _CanSeqAcceptSubParam(const VTID id, const VTParameters& parameters) noexcept;
+
         void _ClearLastChar() noexcept;
     };
 }
diff --git a/src/terminal/parser/stateMachine.cpp b/src/terminal/parser/stateMachine.cpp
index ea455f458d2..eb22ff95382 100644
--- a/src/terminal/parser/stateMachine.cpp
+++ b/src/terminal/parser/stateMachine.cpp
@@ -16,7 +16,11 @@ StateMachine::StateMachine(std::unique_ptr<IStateMachineEngine> engine, const bo
     _state(VTStates::Ground),
     _trace(Microsoft::Console::VirtualTerminal::ParserTracing()),
     _parameters{},
-    _parameterLimitReached(false),
+    _subParameters{},
+    _subParameterRanges{},
+    _parameterLimitOverflowed(false),
+    _subParameterLimitOverflowed(false),
+    _subParameterCounter(0),
     _oscString{},
     _cachedSequence{ std::nullopt }
 {
@@ -158,38 +162,38 @@ static constexpr bool _isParameterDelimiter(const wchar_t wch) noexcept
 }
 
 // Routine Description:
-// - Determines if a character is "control sequence" beginning indicator.
-//   This immediately follows an escape and signifies a varying length control sequence.
+// - Determines if a character is a delimiter between a parameter and its sub-parameters in an escape sequence.
 // Arguments:
 // - wch - Character to check.
 // Return Value:
 // - True if it is. False if it isn't.
-static constexpr bool _isCsiIndicator(const wchar_t wch) noexcept
+static constexpr bool _isSubParameterDelimiter(const wchar_t wch) noexcept
 {
-    return wch == L'['; // 0x5B
+    return wch == L':'; // 0x3A
 }
 
 // Routine Description:
-// - Determines if a character is a private range marker for a control sequence.
-//   Private range markers indicate vendor-specific behavior.
+// - Determines if a character is a "control sequence" beginning indicator.
+//   This immediately follows an escape and signifies a varying length control sequence.
 // Arguments:
 // - wch - Character to check.
 // Return Value:
 // - True if it is. False if it isn't.
-static constexpr bool _isCsiPrivateMarker(const wchar_t wch) noexcept
+static constexpr bool _isCsiIndicator(const wchar_t wch) noexcept
 {
-    return wch == L'<' || wch == L'=' || wch == L'>' || wch == L'?'; // 0x3C - 0x3F
+    return wch == L'['; // 0x5B
 }
 
 // Routine Description:
-// - Determines if a character is invalid in a control sequence
+// - Determines if a character is a private range marker for a control sequence.
+//   Private range markers indicate vendor-specific behavior.
 // Arguments:
 // - wch - Character to check.
 // Return Value:
 // - True if it is. False if it isn't.
-static constexpr bool _isCsiInvalid(const wchar_t wch) noexcept
+static constexpr bool _isCsiPrivateMarker(const wchar_t wch) noexcept
 {
-    return wch == L':'; // 0x3A
+    return wch == L'<' || wch == L'=' || wch == L'>' || wch == L'?'; // 0x3C - 0x3F
 }
 
 // Routine Description:
@@ -201,7 +205,7 @@ static constexpr bool _isCsiInvalid(const wchar_t wch) noexcept
 static constexpr bool _isIntermediateInvalid(const wchar_t wch) noexcept
 {
     // 0x30 - 0x3F
-    return _isNumericParamValue(wch) || _isCsiInvalid(wch) || _isParameterDelimiter(wch) || _isCsiPrivateMarker(wch);
+    return _isNumericParamValue(wch) || _isSubParameterDelimiter(wch) || _isParameterDelimiter(wch) || _isCsiPrivateMarker(wch);
 }
 
 // Routine Description:
@@ -212,8 +216,8 @@ static constexpr bool _isIntermediateInvalid(const wchar_t wch) noexcept
 // - True if it is. False if it isn't.
 static constexpr bool _isParameterInvalid(const wchar_t wch) noexcept
 {
-    // 0x3A, 0x3C - 0x3F
-    return _isCsiInvalid(wch) || _isCsiPrivateMarker(wch);
+    // 0x3C - 0x3F
+    return _isCsiPrivateMarker(wch);
 }
 
 // Routine Description:
@@ -456,7 +460,7 @@ void StateMachine::_ActionVt52EscDispatch(const wchar_t wch)
 
 // Routine Description:
 // - Triggers the CsiDispatch action to indicate that the listener should handle a control sequence.
-//   These sequences perform various API-type commands that can include many parameters.
+//   These sequences perform various API-type commands that can include many parameters and sub parameters.
 // Arguments:
 // - wch - Character to dispatch.
 // Return Value:
@@ -465,7 +469,8 @@ void StateMachine::_ActionCsiDispatch(const wchar_t wch)
 {
     _trace.TraceOnAction(L"CsiDispatch");
     _trace.DispatchSequenceTrace(_SafeExecute([=]() {
-        return _engine->ActionCsiDispatch(_identifier.Finalize(wch), { _parameters.data(), _parameters.size() });
+        return _engine->ActionCsiDispatch(_identifier.Finalize(wch),
+                                          { _parameters, _subParameters, _subParameterRanges });
     }));
 }
 
@@ -495,12 +500,14 @@ void StateMachine::_ActionParam(const wchar_t wch)
     _trace.TraceOnAction(L"Param");
 
     // Once we've reached the parameter limit, additional parameters are ignored.
-    if (!_parameterLimitReached)
+    if (!_parameterLimitOverflowed)
     {
         // If we have no parameters and we're about to add one, get the next value ready here.
         if (_parameters.empty())
         {
             _parameters.push_back({});
+            const auto rangeStart = gsl::narrow_cast<BYTE>(_subParameters.size());
+            _subParameterRanges.push_back({ rangeStart, rangeStart });
         }
 
         // On a delimiter, increase the number of params we've seen.
@@ -513,12 +520,16 @@ void StateMachine::_ActionParam(const wchar_t wch)
             // indicate that further parameter characters should be ignored.
             if (_parameters.size() >= MAX_PARAMETER_COUNT)
             {
-                _parameterLimitReached = true;
+                _parameterLimitOverflowed = true;
             }
             else
             {
                 // Otherwise move to next param.
                 _parameters.push_back({});
+                _subParameterCounter = 0;
+                _subParameterLimitOverflowed = false;
+                const auto rangeStart = gsl::narrow_cast<BYTE>(_subParameters.size());
+                _subParameterRanges.push_back({ rangeStart, rangeStart });
             }
         }
         else
@@ -532,6 +543,62 @@ void StateMachine::_ActionParam(const wchar_t wch)
     }
 }
 
+// Routine Description:
+// - Triggers the SubParam action to indicate that the state machine should
+//   store this character as a part of a sub-parameter to a control sequence.
+// Arguments:
+// - wch - Character to dispatch.
+// Return Value:
+// - <none>
+void StateMachine::_ActionSubParam(const wchar_t wch)
+{
+    _trace.TraceOnAction(L"SubParam");
+
+    // Once we've reached the sub parameter limit, sub parameters are ignored.
+    if (!_subParameterLimitOverflowed)
+    {
+        // If we have no parameters and we're about to add a sub parameter, add an empty parameter here.
+        if (_parameters.empty())
+        {
+            _parameters.push_back({});
+            const auto rangeStart = gsl::narrow_cast<BYTE>(_subParameters.size());
+            _subParameterRanges.push_back({ rangeStart, rangeStart });
+        }
+
+        // On a delimiter, increase the number of sub params we've seen.
+        // "Empty" sub params should still count as a sub param -
+        //      eg "\x1b[0:::m" should be three sub params
+        if (_isSubParameterDelimiter(wch))
+        {
+            // If we receive a delimiter after we've already accumulated the
+            // maximum allowed sub parameters for the parameter, then we need to
+            // set a flag to indicate that further sub parameter characters
+            // should be ignored.
+            if (_subParameterCounter >= MAX_SUBPARAMETER_COUNT)
+            {
+                _subParameterLimitOverflowed = true;
+            }
+            else
+            {
+                // Otherwise move to next sub-param.
+                _subParameters.push_back({});
+                // increment current range's end index.
+                _subParameterRanges.back().second++;
+                // increment counter
+                _subParameterCounter++;
+            }
+        }
+        else
+        {
+            // Accumulate the character given into the last (current) sub-parameter.
+            // If the value hasn't been initialized yet, it'll start as 0.
+            auto currentSubParameter = _subParameters.back().value_or(0);
+            _AccumulateTo(wch, currentSubParameter);
+            _subParameters.back() = currentSubParameter;
+        }
+    }
+}
+
 // Routine Description:
 // - Triggers the Clear action to indicate that the state machine should erase all internal state.
 // Arguments:
@@ -546,7 +613,12 @@ void StateMachine::_ActionClear()
     _identifier.Clear();
 
     _parameters.clear();
-    _parameterLimitReached = false;
+    _parameterLimitOverflowed = false;
+
+    _subParameters.clear();
+    _subParameterRanges.clear();
+    _subParameterCounter = 0;
+    _subParameterLimitOverflowed = false;
 
     _oscString.clear();
     _oscParameter = 0;
@@ -559,7 +631,7 @@ void StateMachine::_ActionClear()
 // Routine Description:
 // - Triggers the Ignore action to indicate that the state machine should eat this character and say nothing.
 // Arguments:
-// - wch - Character to dispatch.
+// - <none>
 // Return Value:
 // - <none>
 void StateMachine::_ActionIgnore() noexcept
@@ -749,6 +821,20 @@ void StateMachine::_EnterCsiParam() noexcept
     _trace.TraceStateChange(L"CsiParam");
 }
 
+// Routine Description:
+// - Moves the state machine into the CsiSubParam state.
+//   This state is entered:
+//   1. When valid sub parameter characters are detected in CsiParam state.
+// Arguments:
+// - <none>
+// Return Value:
+// - <none>
+void StateMachine::_EnterCsiSubParam() noexcept
+{
+    _state = VTStates::CsiSubParam;
+    _trace.TraceStateChange(L"CsiSubParam");
+}
+
 // Routine Description:
 // - Moves the state machine into the CsiIgnore state.
 //   This state is entered:
@@ -1120,8 +1206,8 @@ void StateMachine::_EventEscapeIntermediate(const wchar_t wch)
 //   1. Execute C0 control characters
 //   2. Ignore Delete characters
 //   3. Collect Intermediate characters
-//   4. Begin to ignore all remaining parameters when an invalid character is detected (CsiIgnore)
-//   5. Store parameter data
+//   4. Store parameter data
+//   5. Store sub parameter data
 //   6. Collect Control Sequence Private markers
 //   7. Dispatch a control sequence with parameters for action
 // Arguments:
@@ -1144,15 +1230,16 @@ void StateMachine::_EventCsiEntry(const wchar_t wch)
         _ActionCollect(wch);
         _EnterCsiIntermediate();
     }
-    else if (_isCsiInvalid(wch))
-    {
-        _EnterCsiIgnore();
-    }
     else if (_isNumericParamValue(wch) || _isParameterDelimiter(wch))
     {
         _ActionParam(wch);
         _EnterCsiParam();
     }
+    else if (_isSubParameterDelimiter(wch))
+    {
+        _ActionSubParam(wch);
+        _EnterCsiSubParam();
+    }
     else if (_isCsiPrivateMarker(wch))
     {
         _ActionCollect(wch);
@@ -1250,7 +1337,8 @@ void StateMachine::_EventCsiIgnore(const wchar_t wch)
 //   3. Collect Intermediate characters
 //   4. Begin to ignore all remaining parameters when an invalid character is detected (CsiIgnore)
 //   5. Store parameter data
-//   6. Dispatch a control sequence with parameters for action
+//   6. Store sub parameter data
+//   7. Dispatch a control sequence with parameters for action
 // Arguments:
 // - wch - Character that triggered the event
 // Return Value:
@@ -1270,6 +1358,62 @@ void StateMachine::_EventCsiParam(const wchar_t wch)
     {
         _ActionParam(wch);
     }
+    else if (_isSubParameterDelimiter(wch))
+    {
+        _ActionSubParam(wch);
+        _EnterCsiSubParam();
+    }
+    else if (_isIntermediate(wch))
+    {
+        _ActionCollect(wch);
+        _EnterCsiIntermediate();
+    }
+    else if (_isParameterInvalid(wch))
+    {
+        _EnterCsiIgnore();
+    }
+    else
+    {
+        _ActionCsiDispatch(wch);
+        _EnterGround();
+        _ExecuteCsiCompleteCallback();
+    }
+}
+
+// Routine Description:
+// - Processes a character event into an Action that occurs while in the CsiSubParam state.
+//   Events in this state will:
+//   1. Execute C0 control characters
+//   2. Ignore Delete characters
+//   3. Store sub parameter data
+//   4. Store parameter data
+//   5. Collect Intermediate characters for parameter.
+//   6. Begin to ignore all remaining parameters when an invalid character is detected (CsiIgnore)
+//   7. Dispatch a control sequence with parameters for action
+// Arguments:
+// - wch - Character that triggered the event
+// Return Value:
+// - <none>
+void StateMachine::_EventCsiSubParam(const wchar_t wch)
+{
+    _trace.TraceOnEvent(L"CsiSubParam");
+    if (_isC0Code(wch))
+    {
+        _ActionExecute(wch);
+    }
+    else if (_isDelete(wch))
+    {
+        _ActionIgnore();
+    }
+    else if (_isNumericParamValue(wch) || _isSubParameterDelimiter(wch))
+    {
+        _ActionSubParam(wch);
+    }
+    else if (_isParameterDelimiter(wch))
+    {
+        _ActionParam(wch);
+        _EnterCsiParam();
+    }
     else if (_isIntermediate(wch))
     {
         _ActionCollect(wch);
@@ -1403,7 +1547,7 @@ void StateMachine::_EventSs3Entry(const wchar_t wch)
     {
         _ActionIgnore();
     }
-    else if (_isCsiInvalid(wch))
+    else if (_isSubParameterDelimiter(wch))
     {
         // It's safe for us to go into the CSI ignore here, because both SS3 and
         //      CSI sequences ignore characters the same way.
@@ -1448,7 +1592,7 @@ void StateMachine::_EventSs3Param(const wchar_t wch)
     {
         _ActionParam(wch);
     }
-    else if (_isParameterInvalid(wch))
+    else if (_isParameterInvalid(wch) || _isSubParameterDelimiter(wch))
     {
         _EnterCsiIgnore();
     }
@@ -1521,7 +1665,7 @@ void StateMachine::_EventDcsEntry(const wchar_t wch)
     {
         _ActionIgnore();
     }
-    else if (_isCsiInvalid(wch))
+    else if (_isSubParameterDelimiter(wch))
     {
         _EnterDcsIgnore();
     }
@@ -1625,7 +1769,7 @@ void StateMachine::_EventDcsParam(const wchar_t wch)
         _ActionCollect(wch);
         _EnterDcsIntermediate();
     }
-    else if (_isParameterInvalid(wch))
+    else if (_isParameterInvalid(wch) || _isSubParameterDelimiter(wch))
     {
         _EnterDcsIgnore();
     }
@@ -1737,6 +1881,8 @@ void StateMachine::ProcessCharacter(const wchar_t wch)
             return _EventCsiIgnore(wch);
         case VTStates::CsiParam:
             return _EventCsiParam(wch);
+        case VTStates::CsiSubParam:
+            return _EventCsiSubParam(wch);
         case VTStates::OscParam:
             return _EventOscParam(wch);
         case VTStates::OscString:
@@ -2028,6 +2174,7 @@ void StateMachine::ProcessString(const std::wstring_view string)
             case VTStates::CsiIntermediate:
             case VTStates::CsiIgnore:
             case VTStates::CsiParam:
+            case VTStates::CsiSubParam:
                 _ActionCsiDispatch(*wchIter);
                 break;
             case VTStates::OscParam:
diff --git a/src/terminal/parser/stateMachine.hpp b/src/terminal/parser/stateMachine.hpp
index 4023299c0c6..01976b21b23 100644
--- a/src/terminal/parser/stateMachine.hpp
+++ b/src/terminal/parser/stateMachine.hpp
@@ -31,6 +31,12 @@ namespace Microsoft::Console::VirtualTerminal
     // that number.
     constexpr size_t MAX_PARAMETER_COUNT = 32;
 
+    // Sub parameter limit for each parameter.
+    constexpr size_t MAX_SUBPARAMETER_COUNT = 6;
+    // we limit ourself to 256 sub parameters because we use bytes to store
+    // the their indexes.
+    static_assert(MAX_PARAMETER_COUNT * MAX_SUBPARAMETER_COUNT <= 256);
+
     class StateMachine final
     {
 #ifdef UNIT_TESTING
@@ -85,6 +91,7 @@ namespace Microsoft::Console::VirtualTerminal
         void _ActionVt52EscDispatch(const wchar_t wch);
         void _ActionCollect(const wchar_t wch) noexcept;
         void _ActionParam(const wchar_t wch);
+        void _ActionSubParam(const wchar_t wch);
         void _ActionCsiDispatch(const wchar_t wch);
         void _ActionOscParam(const wchar_t wch) noexcept;
         void _ActionOscPut(const wchar_t wch);
@@ -101,6 +108,7 @@ namespace Microsoft::Console::VirtualTerminal
         void _EnterEscapeIntermediate() noexcept;
         void _EnterCsiEntry();
         void _EnterCsiParam() noexcept;
+        void _EnterCsiSubParam() noexcept;
         void _EnterCsiIgnore() noexcept;
         void _EnterCsiIntermediate() noexcept;
         void _EnterOscParam() noexcept;
@@ -123,6 +131,7 @@ namespace Microsoft::Console::VirtualTerminal
         void _EventCsiIntermediate(const wchar_t wch);
         void _EventCsiIgnore(const wchar_t wch);
         void _EventCsiParam(const wchar_t wch);
+        void _EventCsiSubParam(const wchar_t wch);
         void _EventOscParam(const wchar_t wch) noexcept;
         void _EventOscString(const wchar_t wch);
         void _EventOscTermination(const wchar_t wch);
@@ -152,6 +161,7 @@ namespace Microsoft::Console::VirtualTerminal
             CsiIntermediate,
             CsiIgnore,
             CsiParam,
+            CsiSubParam,
             OscParam,
             OscString,
             OscTermination,
@@ -190,7 +200,11 @@ namespace Microsoft::Console::VirtualTerminal
 
         VTIDBuilder _identifier;
         std::vector<VTParameter> _parameters;
-        bool _parameterLimitReached;
+        bool _parameterLimitOverflowed;
+        std::vector<VTParameter> _subParameters;
+        std::vector<std::pair<BYTE /*range start*/, BYTE /*range end*/>> _subParameterRanges;
+        bool _subParameterLimitOverflowed;
+        BYTE _subParameterCounter;
 
         std::wstring _oscString;
         VTInt _oscParameter;
diff --git a/src/terminal/parser/ut_parser/OutputEngineTest.cpp b/src/terminal/parser/ut_parser/OutputEngineTest.cpp
index db3a6f14588..9695c80936f 100644
--- a/src/terminal/parser/ut_parser/OutputEngineTest.cpp
+++ b/src/terminal/parser/ut_parser/OutputEngineTest.cpp
@@ -391,24 +391,148 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final
         VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground);
     }
 
-    TEST_METHOD(TestCsiIgnore)
+    TEST_METHOD(TestCsiSubParam)
     {
         auto dispatch = std::make_unique<DummyDispatch>();
         auto engine = std::make_unique<OutputStateMachineEngine>(std::move(dispatch));
         StateMachine mach(std::move(engine));
 
+        // "\e[:3;9:5::8J"
         VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground);
         mach.ProcessCharacter(AsciiChars::ESC);
         VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Escape);
         mach.ProcessCharacter(L'[');
         VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiEntry);
         mach.ProcessCharacter(L':');
-        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiIgnore);
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
         mach.ProcessCharacter(L'3');
-        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiIgnore);
-        mach.ProcessCharacter(L'q');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
+        mach.ProcessCharacter(L';');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiParam);
+        mach.ProcessCharacter(L'9');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiParam);
+        mach.ProcessCharacter(L':');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
+        mach.ProcessCharacter(L'5');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
+        mach.ProcessCharacter(L':');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
+        mach.ProcessCharacter(L':');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
+        mach.ProcessCharacter(L'8');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
+        mach.ProcessCharacter(L'J');
         VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground);
 
+        VERIFY_ARE_EQUAL(mach._parameters.size(), 2u);
+        VERIFY_IS_FALSE(mach._parameters.at(0).has_value());
+        VERIFY_ARE_EQUAL(mach._parameters.at(1), 9);
+
+        VERIFY_ARE_EQUAL(mach._subParameters.size(), 4u);
+        VERIFY_ARE_EQUAL(mach._subParameters.at(0), 3);
+        VERIFY_ARE_EQUAL(mach._subParameters.at(1), 5);
+        VERIFY_IS_FALSE(mach._subParameters.at(2).has_value());
+        VERIFY_ARE_EQUAL(mach._subParameters.at(3), 8);
+
+        VERIFY_ARE_EQUAL(mach._subParameterRanges.at(0).first, 0);
+        VERIFY_ARE_EQUAL(mach._subParameterRanges.at(0).second, 1);
+        VERIFY_ARE_EQUAL(mach._subParameterRanges.at(1).first, 1);
+        VERIFY_ARE_EQUAL(mach._subParameterRanges.at(1).second, 4);
+
+        VERIFY_ARE_EQUAL(mach._subParameterRanges.size(), mach._parameters.size());
+        VERIFY_IS_TRUE(
+            (mach._subParameterRanges.back().second == mach._subParameters.size() - 1) // lastIndex
+            || (mach._subParameterRanges.back().second == mach._subParameters.size()) // or lastIndex + 1
+        );
+    }
+
+    TEST_METHOD(TestCsiMaxSubParamCount)
+    {
+        auto dispatch = std::make_unique<DummyDispatch>();
+        auto engine = std::make_unique<OutputStateMachineEngine>(std::move(dispatch));
+        StateMachine mach(std::move(engine));
+
+        Log::Comment(L"Output two parameters with 100 sub parameters each");
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground);
+        mach.ProcessCharacter(AsciiChars::ESC);
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Escape);
+        mach.ProcessCharacter(L'[');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiEntry);
+        for (size_t nParam = 0; nParam < 2; nParam++)
+        {
+            mach.ProcessCharacter(L'3');
+            VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiParam);
+            for (size_t i = 0; i < 100; i++)
+            {
+                mach.ProcessCharacter(L':');
+                VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
+                mach.ProcessCharacter(L'0' + i % 10);
+                VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
+            }
+            Log::Comment(L"Receiving 100 sub parameters should set the overflow flag");
+            VERIFY_IS_TRUE(mach._subParameterLimitOverflowed);
+            mach.ProcessCharacter(L';');
+            VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiParam);
+            VERIFY_IS_FALSE(mach._subParameterLimitOverflowed);
+        }
+        mach.ProcessCharacter(L'J');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground);
+
+        Log::Comment(L"Only MAX_SUBPARAMETER_COUNT (6) sub parameters should be stored for each parameter");
+        VERIFY_ARE_EQUAL(mach._subParameters.size(), 12u);
+
+        // Verify that first 6 sub parameters are stored for each parameter.
+        // subParameters = { 0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5 }
+        for (size_t i = 0; i < 12; i++)
+        {
+            VERIFY_IS_TRUE(mach._subParameters.at(i).has_value());
+            VERIFY_ARE_EQUAL(mach._subParameters.at(i).value(), gsl::narrow_cast<VTInt>(i % 6));
+        }
+
+        auto firstRange = mach._subParameterRanges.at(0);
+        auto secondRange = mach._subParameterRanges.at(1);
+        VERIFY_ARE_EQUAL(firstRange.first, 0);
+        VERIFY_ARE_EQUAL(firstRange.second, 6);
+        VERIFY_ARE_EQUAL(secondRange.first, 6);
+        VERIFY_ARE_EQUAL(secondRange.second, 12);
+    }
+
+    TEST_METHOD(TestLeadingZeroCsiSubParam)
+    {
+        auto dispatch = std::make_unique<DummyDispatch>();
+        auto engine = std::make_unique<OutputStateMachineEngine>(std::move(dispatch));
+        StateMachine mach(std::move(engine));
+
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground);
+        mach.ProcessCharacter(AsciiChars::ESC);
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Escape);
+        mach.ProcessCharacter(L'[');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiEntry);
+        mach.ProcessCharacter(L'3');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiParam);
+        mach.ProcessCharacter(L':');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
+        for (auto i = 0; i < 50; i++) // Any number of leading zeros should be supported
+        {
+            mach.ProcessCharacter(L'0');
+            VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
+        }
+        for (auto i = 0; i < 5; i++) // We're only expecting to be able to keep 5 digits max
+        {
+            mach.ProcessCharacter((wchar_t)(L'1' + i));
+            VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiSubParam);
+        }
+        VERIFY_ARE_EQUAL(mach._subParameters.back(), 12345);
+        mach.ProcessCharacter(L'J');
+        VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Ground);
+    }
+
+    TEST_METHOD(TestCsiIgnore)
+    {
+        auto dispatch = std::make_unique<DummyDispatch>();
+        auto engine = std::make_unique<OutputStateMachineEngine>(std::move(dispatch));
+        StateMachine mach(std::move(engine));
+
         mach.ProcessCharacter(AsciiChars::ESC);
         VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::Escape);
         mach.ProcessCharacter(L'[');
@@ -417,7 +541,7 @@ class Microsoft::Console::VirtualTerminal::OutputEngineTest final
         VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiParam);
         mach.ProcessCharacter(L';');
         VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiParam);
-        mach.ProcessCharacter(L':');
+        mach.ProcessCharacter(L'=');
         VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiIgnore);
         mach.ProcessCharacter(L'8');
         VERIFY_ARE_EQUAL(mach._state, StateMachine::VTStates::CsiIgnore);


Test patch: