Add support for the VSCode mark sequences*

[zadjii-msft]

*that we already support

VsCode has their own fork of the FTCS marks. Theirs uses 633 as the OSC prefix instead of 133. This adds support to the Terminal for that format, as an alias for the FTCS marks we already support.

This does not add any of the other sequences we didn't already support.

see: #11000
as complained about in #7434

[DHowett]

curious, did we merge this without code format somehow?

[zadjii-msft]

https://github.com/microsoft/terminal/blame/6873c85d2d8c818c2c5ef99971f370c447ebd2de/src/terminal/parser/OutputStateMachineEngine.cpp#L419 seems to show that this came from #15648, and that format passed.

I honestly have no idea why it thinks both are right?

[lhecker]

Instead of adding another interface method, we can also do this:

case OscActionCodes::FinalTermAction:
case OscActionCodes::XtermJsAction:
    success = _dispatch->DoFinalTermAction(string);
    break;

While I understand that they aren't ideologically the same thing, they're semantically identical at the moment. As such I believe that the split has no "tangible" benefit right now and I think it would be better without it (in other similar places as well for the same reason).

[DHowett]

That would make this review so so much shorter...!

[j4james]

I think this is a bad idea. They specifically changed their shell integration to use a different OSC sequence because they intended it to be incompatible with OSC 133, and didn't want there to be conflicts with shell scripts using the standard FTCS format (see microsoft/vscode#140514 (comment)). If we start implementing OSC 633 as an alias of OSC 133 that is just going to encourage people to treat them as the same thing, which defeats the whole purpose of the VSCode fork.

[DHowett]

I'm blocking to make sure we hear James' concerns.

My personal bent is... they intended to do something incompatible, but now they will have scripts in the wild relying on behaviors that are fully compatible for A through D. I think we'd be remiss in not handling those the same . . . and if they ever come out with things that diverge, two things happen:

1. They are not A-D, and we ignore them (safely).
2. They change A-D and break existing scripts and also Terminal.

Whether we implement those operations by funneling them through the FinalTerm handler doesn't actually matter, because no matter what sort of breaking change they make we would need to make a change on our side to support it. All that changes is the size of the diff--we separate out the handlers then, or we already have them separated.

(Unless you're recommending not supporting them at all, in which case this argument has no bearing. 😄)

[j4james]

I'm recommend you don't support them at all. At the very least I think you should contact the VSCode devs and ask them what their intentions are for these sequences.

Because the problem, as I understand it, is that apps don't like having to check whether they should be using OSC 133 or OSC 633. Either those two sequences are incompatible, in which case the check is necessary, and us creating an alias is just going to break things. Or they are actually aliases (at least for the primary A-D commands), in which case the obvious fix is for VSCode to add back support for OSC 133 and everyone can just use that. ¹

Right now I'm assuming there are apps or scripts doing something like this:

if terminal == vscode:
  use_osc633
else:
  use_osc133

So how is this PR going to improve thing? Are you imagining that's going to be better as:

if terminal == vscode or terminal == windowsterminal:
  use_osc_633()
else:
  use_osc_133()

Because that does not look like an improvement to me, and it's not feasible anyway, because there's no real way to detect Windows Terminal. So what you're really going to encourage is this:

use_osc_633()

And then every other terminal that was doing the right thing, and sticking with the existing standard, is now going to be shafted.

Footnotes

1. I was also under the impression that VSCode had already added back support for OSC 133, in which case there's no need for scripts to use anything else, unless they specifically required OSC 633 functionality that is different from OSC 133. ↩

[zadjii-msft]

At the very least I think you should contact the VSCode devs

We can do that! @Tyriar want to chime in here?

---

My intention on having these be separate methods on the ITermDispatch layer was to make sure that in the future, when we do want to support the variations between these two sequences, we could. It was just a handy for now that the parts of each that we support did exactly the same thing. I think it's a reasonable point though - if there's no functional difference between A-D, then the 633 variants really shouldn't exist

[j4james]

if there's no functional difference between A-D, then the 633 variants really shouldn't exist

Exactly! And if is that is the case, then I think it's our responsibility to discourage their use as much as possible, for the benefit of the wider terminal community. That means not implementing them unless there's a very compelling use case that can't be addressed in any other way.

[Tyriar]

Exactly! And if is that is the case, then I think it's our responsibility to discourage their use as much as possible, for the benefit of the wider terminal community. That means not implementing them unless there's a very compelling use case that can't be addressed in any other way.

I agree with @j4james that there's no point implementing it unless/until you:

• Ship your own shell integration scripts
• Implement the extended sequences (currently 633 E or P)

---

At the very least I think you should contact the VSCode devs and ask them what their intentions are for these sequences.

Here's the rationale for why they exist:

VS Code-specific shell integration sequences. Some of these are based on more common alternatives
like those pioneered in FinalTerm. The decision to move to entirely custom sequences was to try
to improve reliability and prevent the possibility of applications confusing the terminal. If
multiple shell integration scripts run, VS Code will prioritize the VS Code-specific ones.

It's recommended that authors of shell integration scripts use the common sequences (eg. 133)
when building general purpose scripts and the VS Code-specific (633) when targeting only VS Code
or when there are no other alternatives.

https://github.com/microsoft/vscode/blob/e8f9d7587b45626fa8bb5a71b749c713f46e93c4/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts#L61-L71

Something missing from that description is they could also provide additional information in the future and I absolutely did not want to try change what the 133 ones are meant to mean.

Additionally, in VS Code the 633 A/B/C/D sequences are used as an indicator that full shell integration has been activated for diagnostic purposes; ie. if 633 A exists, it's safe to assume that the shell implements "VS Code's shell integration" to the best of its ability (run recent command, go to directory etc.), rather than fallback (basically only decorations and command navigation).

I'm not sure if we currently surface this, but the idea was to enable additional information to be reported here:

Currently I think the Julia VS Code extension and Nushell (when running in VS Code) are the only implementors of 633 outside the first party scripts.

---

TLDR: I wouldn't implement this unless you start shipping your own shell integration scripts that fully support OSC 633.

[Tyriar]

I'd call them VS Code sequences as they're not built into xterm.js or mentioned anywhere in the repo

[zadjii-msft]

Ah good note! I assumed that everything terminal-parsing related was in xterm.js itself.

[zadjii-msft]

Okay we're all in agreement. 133 is what CLI apps should generally implement.

I'll probably resurrect this at some point in the future, because 633;E is well, pretty dang handy for the edge cases. But no point in supporting the others until then.

[j4james]

I'll probably resurrect this at some point in the future, because 633;E is well, pretty dang handy for the edge cases.

If we do eventually decide to do this, be aware that 633;E is not compatible with 133;E, and I suspect the same is probably true of 633;P.

[Tyriar]

@j4james there is no 133 E or P, is there?

[zadjii-msft]

If we do eventually decide to do this, be aware that 633;E is not compatible with 133;E, and I suspect the same is probably true of 633;P.

Oh, for sure. That was always going to be the point I forked the implementations back to separate methods.

[j4james]

@Tyriar In the original Final Term implementation, 133;E and 133;F were used for marking up things like file names and process IDs. And 133;P is an extension from the terminal-wg semantic prompts proposal. I believe it was originally implemented in DomTerm, but I think there are other terminals that support it now as well. It would have been nice if VSCode had done the same thing, and just proposed the functionality you needed as OSC 133 extensions, instead of a whole new sequence that looks almost, but not quite like an alias of OSC 133. It's not too late to do that now. It would be a whole lot less confusing for everyone.

[zadjii-msft]

from the terminal-wg semantic prompts proposal

for the lazy: https://gitlab.freedesktop.org/terminal-wg/specifications/-/issues/4

[Tyriar]

I believe it was originally implemented in DomTerm, but I think there are other terminals that support it now as well. It would have been nice if VSCode had done the same thing, and just proposed the functionality you needed as OSC 133 extensions, instead of a whole new sequence that looks almost, but not quite like an alias of OSC 133.

@j4james I'm not so sure about this.

Personally I think it's far better to go with our own explicit vscode sequences instead of trying to modify an ancient de-facto standard like OSC 133 with little to no input/consensus from the community. The latter just makes it ambiguous what you're meant to do, whereas OSC 633 is clearly defined and is generally only meant to be implemented inside and for VS Code/MS. Additionally, we needed functionality beyond what the ones you're pointing and terminal-wg was a failed experiment imo.

As for 133;A-D, you have a point and it was discussed at length in the vscode issues and the result of that discussion was to add support for 133 as a fallback. Perhaps I made a bad decision there, we could change it but tbh I don't think it's a problem at all currently when it's typically only first party/MS implementing these, with a very short list of external implementors; ie. it's essentially a proprietary protocol. It could very well become a de-facto standard in the future, but right now we're just doing our own thing in order to move fast and enable features that we've wanted for a long time but could not using standard sequences.

Personally I kind of like the model where different terminals pioneer sequences/features in their own little part of the world and they eventually gain wider acceptance, like Final Term's 133, iTerm2's 1337 and kitty's extended underline.

[j4james]

OSC 633 is clearly defined and is generally only meant to be implemented inside and for VS Code/MS

That's a fair point. I think the main issue was the overlap between 133;A-D and 633;A-D. If it had just been your proprietary extensions in a new sequence it wouldn't have bothered me as much.

Additionally, we needed functionality beyond what the ones you're pointing and terminal-wg was a failed experiment imo.

Yeah, I don't think terminal-wg has any hope of developing new standards, but I do still think it can be useful for terminals to share their plans. For example, if you're planning on implementing a new OSC sequence, announcing your intention on terminal-wg can help avoid conflicts with other terminals that might already be using the same number.

And in the case of OSC 133, it would be a way to let everyone know that you're thinking of using say command P for a new purpose. And hopefully someone would then let you know if that was going to conflict, or there's perhaps already a command that others are using that you could adopt instead. You don't necessarily need everyone to agree that what you're doing is going to become a new standard, but at least there's a better chance of some minimal cooperation that way.

but right now we're just doing our own thing in order to move fast and enable features that we've wanted for a long time but could not using standard sequences.

Fair enough.