Add support for ITU's T.416 - ODA SGR (38/48) sequence

[tusharsnx]

This PR adds support for ITU's T.416 - ODA SGR (38/48) colour sequence, which makes use of colon instead of semi-colon as a parameter separator.

• We use semi-colons as the only parameter separator while sending SGR color sequences to a ConPTY client. This is to keep backward compatibility.
• In response to DECRQSS query, we have decided to use colons, as the major usecase for such queries are feature detection (whether client supports ODA colours), and tracking the original separator may add too much complexity to the codebase.

Validation Steps Performed

• Made sure that we are always sending semi-colon separated parameters regardless of whether the original sequence used colons.
• Made sure that we are always using colons as the parameter separator in a DECRQSS response.
• Added new tests!

Closes #15706

[tusharsnx]

I feel weird about adding new secret color types that indicate color shapes

Yeah, totally makes sense. And don't worry, this is not the final look of the PR 😄. I put it up so I get some early reviews and suggestion on how we'd like to go about implementing this.
The only reason I gave this implementation a shot was, maybe it'll better if I put the color and separator info together. But it seems it causes more confusion than any good to us 😅

I have few alternative ideas:

Separator info inside TextBuffer.

In this case, we can track the last received separator in any color sequence within TextBuffer. This should generally work as intended, though I need to check if I can implement this without any issues. As the only reason to store separator info is to be able to response back on queries with right separator, this should work fine.

Separator info inside TextAttribute.

If we keep it inside TextAttribute, maybe within _attr, which holds other attribute flags, there are two unused flags in it. We can use them to get the same effect as Index256Colon, IsRgbColon. We would need both the flags, each for Fg and Bg colour.

Let me know, which implementation sounds better to you 😀

[lhecker]

I think it's fine to put the separator info into TextColor. We have COLORREF and til::color for all other, regular colors after all. 🙂

But I think I have an idea how to simplify it a bit. Instead of adding multiple new ColorTypes I believe you can add a boolean flag to the struct. To save space (we store a lot of TextAttributes after all), we can use a bitfield:

struct TextColor
{
    // ...

private:
    union
    {
        BYTE _red, _index;
    };
    BYTE _green;
    BYTE _blue;
    ColorType _meta : 7;
    BYTE _isColon : 1 = 0;
};

Now we can properly store it as if it was a boolean and simplify SetColon().
Additionally, instead of having a IsColon() function I would use a GetSeparator() one that returns a char - either ';' or ':'. This is IMO better than IsColon() because it's simply more practical: The meaning of what "separator" refers to will never change and it simplifies some code around the code base. If we ever do end up needing a IsColon() function we can just add it. With that in place we can write this:

const auto sep = color.GetSeparator();
fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{}{2}5{2}{}"), base + 8, index, sep);

(If you write {} {} {} then fmt::format will implicitly assume {0} {1} {2}. The {2} means that it will explicitly pick the 2nd (0-based index) variadic argument.)

[DHowett]

(If you write {} {} {} then fmt::format will implicitly assume {0} {1} {2}. The {2} means that it will explicitly pick the 2nd (0-based index) variadic argument.)

FWIW, exposing the separator as a character will give us a false sense of equivalence.

It is not as simple as replacing ; with : in the report. The CSI sequence with subparameters is materially different:

Compatibility CSI 38 ; 2 ; Pr ; Pg ; Pb m
T.416 ODA     CSI 38 : 2 : Pi : Pr : Pg : Pb m

It just so happens that Pi (color space identifier) is empty and ignored.
So in reality, it looks like CSI 38:2::R:G:B m with the double colon.

I would swing the other way and call it IsT416ODAColor even though it is a mouthful. It encapsulates what we are actually tracking, not what it happened to look like at the time.

[lhecker]

Oh I didn't notice that there are two :: in the one branch. I thought they're identical apart from the separator. In that case, please ignore the second half of my previous comment. 😅

[DHowett]

I'm super cool with the bitfield version though - that's clever!

[tusharsnx]

To save space (we store a lot of TextAttributes after all), we can use a bitfield.

ColorType _meta : 7;
BYTE _isColon : 1 = 0;

🤯 that's a TIL for me.

This is definitely a cleaner approach than mine! Thanks.

As @j4james suggested we don't need to track colour type, let's see if that works out for us. If not, we'll go ahead with bitfields approach then.

[lhecker]

Another fantastic PR. Thanks! 🙂

[j4james]

I made a couple of suggestions, but nothing blocking. I'd be quite happy with this as it is.

[DHowett]

I love this! I'm going to give you a little bit to address open feedback before I merge this PR, but thank you so much for doing it.

My only nit -- and it's a small one -- is that I personally prefer SubParam instead of Alt. It makes it clearer when to use which one. I might even go so far as to call it _ApplyGraphicsOptionWithSubParams, _SetRgbColorsHelperFromSubParams

Up to you!

[tusharsnx]

Pushed the requested changes.

I've also included some tests to ensure that we ignore colors if any of the following conditions are true:

• One of the color components (Index, R, G, or B) is out of range.
• The ColorSpaceID is not empty.

[DHowett]

Oh, in case I didn't say it enough: HELL YES, THANK YOU. I've wanted this for EVER.