Add XTVERSION sequence support by aymanbagabas · Pull Request #20268 · microsoft/terminal (original) (raw)

@aymanbagabas

Implements response to CSI > Ps q sequence (XTVERSION) as requested in #18382.

When Ps is 0 (default), the terminal responds with DCS > | text ST identifying the host application and version:

Implementation

Fixes #18382

[github-advanced-security[bot]](/apps/github-advanced-security)

@github-actions

This comment has been minimized.

DHowett

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple thoughts!

Thanks for taking a crack at this.

@DHowett

It's interesting that as GitHub staff (hi!) but not Microsoft staff (as far as I can tell) your PRs automatically trigger our CI pipelines, even though they're set to only build for organization members by default. I think that's something we're going to have to bring to somebody's attention (!)

@aymanbagabas

Hey @DHowett, I am indeed a GitHub and Microsoft staff joined recently and a previous contributor as well. I think that's why CI triggered automatically.

I have updated the changes to fix the spelling check and implement the version package identity on the TerminalApi interface.

j4james

PPR_PagePositionRelative = VTID(" Q"),
PPB_PagePositionBack = VTID(" R"),
DECSCUSR_SetCursorStyle = VTID(" q"),
XT_RequestVersion = VTID(">q"),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This list is meant to be sorted by intermediate/final, and in this case the final is q, so it should ideally be placed between the DSR queries (final n) and DECSTBM (final r).

In a sane world it would have had a c final, and been grouped with the DA queries.

{
const auto package = winrt::Windows::ApplicationModel::Package::Current();
const auto v = package.Id().Version();
return winrt::hstring{ fmt::format(FMT_COMPILE(L"Windows Terminal {}.{}.{}.{}"), v.Major, v.Minor, v.Build, v.Revision) };

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: There's no real standard for this, but it's worth noting that both XTerm and VTE include the version number in parentheses, so if we wanted to match them we should really use something like Windows Terminal(1.25.1322.0).

A lot of terminals just have a space separator, though, so it's not a big deal either way.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. But because there is no standard, and the ones that use the TERM(VERSION) format usually use a single word terminal name. I think Windows Terminal X.Y.Z.W is fine and is a common pattern that applications can understand.
std::wstring_view ConhostInternalGetSet::GetHostIdentity() const
{
return L"Windows Console Host";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For ConHost, could we not perhaps pull a version number from GetFileVersionInfo? Maybe also do something with the module name to differentiate between OpenConsole and inbox ConHost (I suspect they're using completely different version schemes).

If not, I think it might be better to just ignore XTVERSION requests altogether, because returning a name without a version will likely just be used to blacklist ConHost.

@DHowett

ah, that makes sense.

I had to page the discussion from #18382 back in a little bit, and I'm worried that overall this doesn't cover any of the more complex cases that we talked about over there, Terminal as a control hosted inside Visual Studio chief among them. I'm willing to overlook that in the general case...

I am, as always, worried about the application ecosystem's response to this. We already hugely regret the WT_SESSION environment variable, because

@DHowett

oh, @j4james beat me to it. And then I submitted my comment without finishing it.

We regret WT_SESSION to an extreme extent because applications use it to detect Terminal, and therefore they refuse to work properly in the Windows Console. They would work fine there, of course, but developers are lazy and generally propagate poor choices forward. I will note that I saw this coming in 2020 and encouraged developers not to do it. Oh well.

Why should I believe that XTVERSION makes that situation any better?

@j4james

For the record, I'm also very much opposed to adding XTVERSION reporting. We've had years of evidence demonstrating how this sort of thing is abused. The best thing Windows Terminal can do to discourage that behaviour is by refusing to identify itself, and force app to do feature detection the right way.

@aymanbagabas

For the record, I'm also very much opposed to adding XTVERSION reporting. We've had years of evidence demonstrating how this sort of thing is abused. The best thing Windows Terminal can do to discourage that behaviour is by refusing to identify itself, and force app to do feature detection the right way.

I agree. But as a TUI developer, currently, there is no single spec to identify terminal features. Some features can be detected via DECRQM (but not all terminals support that), some require their own sequences like Kitty Keyboard & Graphics protocols. Then you have the ones that you cannot detect reliably like hyperlinks OSC8, clipboard OSC52, notifications OSC9, progress bar OSC 9;4, URxvt extensions OSC 777 (usually used for notifications), OSC 99 (Kitty extended notifications protocol), OSC 5522 (Kitty new clipboard protocol), and so on.
Some terminals like Tmux define their own Terminfo capability extensions to advertise for some of these features. For example, Tc for true color, Hls for hyperlinks, Spb for progress bar, and Ms for clipboard, Smulx for underline extensions, etc.

If you're lucky, your terminal would support XTGETTCAP and also define these Tmux Terminfo extensions in their internal database. Then you would be able to query for these Terminfo capabilities.

Terminal features that use DECSM/DECRM are easy to detect as long as the terminal supports DECRQM. Apple Terminal doesn't and currently bleeds to the terminal a lone "p" character that comes from the query cat | printf '\x1b[?25$p'. The latest Tmux version 3.6a only responds to DECRQM for specific set of modes.

Which brings us to basic detection via $TERM. From a TUI framework PoV, this is the first point for feature detection. In the case of Windows Terminal, I would be looking for $WT_SESSION as well. Then if $TERM_PROGRAM is not Apple Terminal, I would start querying for the above to detect support. If you're behind SSH and advertise SSH_TTY then good luck trying to figure out which terminal is behind without querying the name via XTVERSION (which also bleeds on Apple Terminal).

So, this really brings me to the last resort of detecting features via XTVERSION 🙂

@j4james

So why not ask those terminals to support DECRQM, or DECRQSS, or propose new extensions to DA for missing features? These are things that will automatically gain those terminals additional functionality in existing applications. And applications that determine capabilities using those standards will automatically be compatible with future terminals without even knowing of their existence.

But instead you want to get terminals to support XTVERSION, which gains them nothing, unless they also go around asking every single application using XTVERSION to add their terminal name to the app's database. Then whenever they implement a new feature they've also got to get all those apps to update their databases to reflect that change. And any new terminals that are developed in the future are guaranteed to have downgraded functionality in these applications.

I don't know why I keep having this argument, because I know it never convinces anyone, but I really cannot understand why there are still people think this is a good idea. It seems insane to me.

@aymanbagabas

Implements response to CSI > Ps q sequence (XTVERSION). When Ps is 0 (default), the terminal responds with DCS > | text ST identifying the host application and version.

The host identity is determined by each implementation:

This keeps the adapter layer simple and allows each host to determine its own identity appropriately.

Fixes microsoft#18382

Signed-off-by: Ayman Bagabas ayman.bagabas@gmail.com

@aymanbagabas

So why not ask those terminals to support DECRQM, or DECRQSS, or propose new extensions to DA for missing features? These are things that will automatically gain those terminals additional functionality in existing applications. And applications that determine capabilities using those standards will automatically be compatible with future terminals without even knowing of their existence.

Many are slowly adding support for DECRQM. I don't think proposing new extensions to DA1/2/3 would be a good idea. Almost all terminals support DA1, but not DA2/3. And even if they do, they don't necessarily report modern features via DA. AFAICT, DA1 is usually used to detect Sixel, DRCS, and other DEC terminal features but not newer ones.

But instead you want to get terminals to support XTVERSION, which gains them nothing, unless they also go around asking every single application using XTVERSION to add their terminal name to the app's database. Then whenever they implement a new feature they've also got to get all those apps to update their databases to reflect that change. And any new terminals that are developed in the future are guaranteed to have downgraded functionality in these applications.

Which is exactly why Terminfo databases should be avoided here. How can a TUI application reliably know the terminal its running in? Historically, and via Terminfo databases, that would be detected via $TERM. But nowadays, you cannot reliably detect that. Many terminals just advertise themselves as xterm-256color.

Nowadays, and as you can see in the list linked in the issue, many modern terminals already support XTVERSION. Some missing ones include Ghostty, Rio, and Konsole.

Now with XTVERSION, I think that this is quite valuable information from an application perspective. If I can't detect support via DECRQM, I would fall back to XTVERSION parsing the name and version that I know supports a certain feature.

I don't know why I keep having this argument, because I know it never convinces anyone, but I really cannot understand why there are still people think this is a good idea. It seems insane to me.

That's the whole point of a discussion 😄

@DHowett

@j4james

they don't necessarily report modern features via DA.

OSC 52 support is now reported via DA1 by many terminals, which goes to show that terminal devs are happy to do that sort of thing if you just ask them.

Which is exactly why Terminfo databases should be avoided here.

I wasn't talking about the termininfo databases. I'm talking about the database that an application using XTVERSION has to maintain to determine whether terminal X with version Y supports feature Z. You're just swapping a centrally maintained database for one that every application has to maintain.

Many terminals just advertise themselves as xterm-256color.

Precisely because terminal apps were using the TERM name as a form of feature detection, which is exactly the same thing they're now doing with XTVERSION! Does nobody ever learn from the mistakes of the past?

@aymanbagabas

@aymanbagabas

OSC 52 support is now reported via DA1 by many terminals, which goes to show that terminal devs are happy to do that sort of thing if you just ask them.

That's great! But DA1 specifications were not made to target OSC features. It was designed to query for physical video terminal "architecture class and basic attributes". For example, if we would add OSC8 to that list, we would already have a problem because 8 is reserved for User-Defined Keys (UDKs) attribute.

I wasn't talking about the termininfo databases. I'm talking about the database that an application using XTVERSION has to maintain to determine whether terminal X with version Y supports feature Z. You're just swapping a centrally maintained database for one that every application has to maintain.

Fair. But again, the reason behind XTVERSION is to detect the terminal name and version. What developers do with that information is their responsibility not the terminal's. Sure, some people would abuse that to detect terminal features, but I don't think that by itself is a good reason to not implement it.

Precisely because terminal apps were using the TERM name as a form of feature detection, which is exactly the same thing they're now doing with XTVERSION! Does nobody ever learn from the mistakes of the past?

I agree. But this is different. TERM was designed to be small and short for times where data and bandwidth weren't cheap. Nowadays, querying for richer specifics and details makes more sense. It also works remotely in SSH or TELNET settings without fussing with environment variables and configuring them to be sent to the other end.

I'm not suggesting using XTVERSION to detect feature support. While it is an option, there are many more examples where this would be beneficial. One would be tracking the terminal name and version for telemetry data. You can't get that via TERM reliably and other environment variables and certainly not over remote sessions without querying or explicitly passing environment variables.

@DHowett

I'm not suggesting using XTVERSION to detect feature support.

Unfortunately, I don't think that matters. I was not suggesting using WT_SESSION to detect feature support, yet here we are.

@j4james

if we would add OSC8 to that list, we would already have a problem because 8 is reserved for User-Defined Keys (UDKs) attribute.

The extension numbers don't have any meaning beyond what is assigned to them when the extension is defined. There is no requirement that support for OSC 8 should be identified by extension number 8. For example, DECCOLM (132 Column mode) has a mode number of 3, but is identified by the extension number 1. We chose the number 52 to represent OSC 52 because it happened to be free, and made for a nice match. If I were proposing an extension for OSC 8, I would suggest something like 88 for similar reasons, but it could be anything.

@aymanbagabas

@j4james I’d be very interested in helping extend DA1, or any other established terminal specification, for feature discovery. While XTVERSION can be misused by applications (like other mechanisms), I don’t think that should prevent terminals from being able to identify themselves. Is there a reliable way to identify Windows Terminal over remote sessions? WT_SESSION doesn’t work via SSH.

@DHowett For what it’s worth, internally we use a mix of signals such as TERM, WT_SESSION, TERM_PROGRAM, and other terminal-related environment variables to decide whether we can safely query the terminal. Some terminals, like Apple Terminal, can bleed garbage output and break rendering on certain queries, which can subtly but noticeably disrupt the app view. We also record the detected terminal name for telemetry purposes.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})