as a general FYI, @j4james is pretty much the owner of this whole section of the codebase at this point, so I'd appreciate their S/O

I have some issues with the standards compliance of this implementation.

Per STD 070, section 3.5.3.1, "The bit combination 3/10 is reserved for future standardization. If 3/10 is received within a parameter string, the entire sequence up to and including the final character shall be ignored." If we accept the ODA documentation as the "future standardization", that only defines 3/10 usage in the SGR sequence.¹ Outside of that the original rules still apply.

So something like \e[10:20H should still be a no-op. It should not be interpreted as \e[10H.

It's arguable that colons could now be accepted in controls with selective parameters, but even then, any parameter substring that we don't yet support is the equivalent of an unimplemented parameter. And section 3.5.1.3 says "unimplemented selective parameter values of control sequences shall be ignored as if that parameter were not received."

So something like \e[?5:10h should also be no-op. It should not be interpreted as \e[?5h.

The only thing up for debate is whether something like \e[?10:10;5h should be entirely ignored, or we could just ignore the initial parameter substring. But I'm leaning towards ignoring the entire thing until parameter substrings have defined usage in that particular control sequence.

Then within SGR itself, any parameter substrings that we don't yet recognise are still the equivalent of unimplemented parameters, so they should still be ignored as mentioned above.

So something like \e[31:42m should be a no-op. It should not be interpreted as \e[31m. Even something like \e[4:3m should be a no-op, until we actually support extended underline sequences.

@j4james, what is ODA?

@j4james, what is ODA?

Sorry, I should have been clearer about that. By ODA, I meant the Open Document Architecture format, ITU-T recommendation T.416 or ISO/IEC 8613-6. That's where modern terminals got the idea for the colon-based 256-color and RGB extensions, although it's not actually a terminal standard as such.

@j4james I don't want to pretend that I understood everything what you said there, because I'm too new to all the inner workings of terminal. But I guess what you mean is, incase we don't know if a parameter accepts sub parameters or not, then we should better enter into CsiIgnore state, instead of simply parsing it further. So it's more like a opt-in to accepts sub parameter, isn't it?
I like this idea, this ensures future proofing, since if suddenly there's a new sequence(that uses sub parameter) and we don't how it works, it will be safely discarded within PTY layer (just like how it is now).

Let me see how we can achieve it on this PR. Should that address all your concerns?

So it's more like a opt-in to accepts sub parameter, isn't it?

@tusharsnn That's exactly right, yes. But note that the CsiIgnore state is currently handled at the StateMachine level, but you don't know what the actual control sequence is going to be used for until you've parsed the final character, and then moved on to the OutputStateMachineEngine.

My initial idea was you could have a check at the start of the ActionCsiDispatch method in OutputStateMachineEngine, and that could immediately abort if the parameters contained any sub parameters, while the id was anything other than SGR. You'd also need a similar check in ActionDcsDispatch, except that should always abort if there are any sub parameters.

What I'm not sure about is how we'd handle DECCARA and DECRARA, because only some of their parameters are allowed to have sub parameters. I think that's going to end up being a bit messy.

What I'm not sure about is how we'd handle DECCARA and DECRARA, because only some of their parameters are allowed to have sub parameters. I think that's going to end up being a bit messy.

@j4james Can you share a reference, so that I can have a look on which parameters support sub parameters.

if(_acceptsSubParam(_parameters.back()))

But you can't tell whether a given parameter accepts sub parameters at this point. For example, if the parameter is a 4, it could indicate the underline attribute of an SGR sequence, but it could also be the row number 4 from a CUP sequence, or the insert-replace mode number in an SM or RM sequence, or a thousand other things.

Can you share a reference, so that I can have a look on which parameters support sub parameters.

See https://vt100.net/docs/vt510-rm/DECCARA.html and https://vt100.net/docs/vt510-rm/DECRARA.html.

The first four parameters are numeric parameters, which can't have sub parameters. The remaining parameters are rendition attributes, which are selective parameters, and should theoretically support the same values that we allow in an SGR sequence.

@j4james

But you can't tell whether a given parameter accepts sub parameters at this point.

yeah, you're right. I realised that after I made the reply. right now, I think we should guard against both parameter and sequence to never allow parameter when we don't accept them.

So, something like \x1b[38:2::3m should rightfully enter CsiIgnore right after we parse a : after 38. This should avoid the need to check if a parameter has a sub parameter(s) when it isn't suppose to have one for every parameter in SGR .

I've updated the validation steps to test for two new scenarios.

I don't think the state machine should have any idea of what 38 means or whether it should have subparameters in SGR.

I wanna work a little bit on ergonomics for accessing sub parameters. As it is, to get sub parameter at index 0 from the std::span of sub parameters, we would need to do til::at(subParams, idx). It'll be better if we can reduce it to subParams.at(idx) like how we do for parameters.

I'm thinking of introducing VTSubParameters class for this purpose, and later update VTParameters to hold this instead of plain std::span<const VTParameter>.

It was a great experience working on Windows Terminal for the first time ❤️, feels like an achievement to me. @j4james @zadjii-msft @DHowett @lhecker @carlos-zamora Thank you all for reviewing the PR.