Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No support for xterm Send Device Attributes (Secondary DA) control sequence #5836

Closed
zrait opened this issue May 11, 2020 · 7 comments · Fixed by #6850
Closed

No support for xterm Send Device Attributes (Secondary DA) control sequence #5836

zrait opened this issue May 11, 2020 · 7 comments · Fixed by #6850
Labels
Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@zrait
Copy link

zrait commented May 11, 2020

Environment

Windows build number: [Version 10.0.19041.208]
Windows Terminal version (if applicable): 0.11.1251.0

Steps to reproduce

echo -e '\e[>c'

Expected behavior

A secondary device attribute response providing information that better describes what capabilities Windows Terminal actually supports.

Excerpt from xterm ctl sequences:

CSI > Ps c
          Send Device Attributes (Secondary DA).
            Ps = 0  or omitted ⇒  request the terminal's identification
          code.  The response depends on the decTerminalID resource set-
          ting.  It should apply only to VT220 and up, but xterm extends
          this to VT100.
            ⇒  CSI  > Pp ; Pv ; Pc c
          where Pp denotes the terminal type
            Pp = 0  ⇒  "VT100".
            Pp = 1  ⇒  "VT220".
            Pp = 2  ⇒  "VT240" or "VT241".
            Pp = 1 8  ⇒  "VT330".
            Pp = 1 9  ⇒  "VT340".
            Pp = 2 4  ⇒  "VT320".
            Pp = 3 2  ⇒  "VT382".
            Pp = 4 1  ⇒  "VT420".
            Pp = 6 1  ⇒  "VT510".
            Pp = 6 4  ⇒  "VT520".
            Pp = 6 5  ⇒  "VT525".

          and Pv is the firmware version (for xterm, this was originally
          the XFree86 patch number, starting with 95).  In a DEC termi-
          nal, Pc indicates the ROM cartridge registration number and is
          always zero.

Actual behavior

No response to control sequence

Why this matters right now?

When Windows Terminal sets TERM to be xterm-256color, certain popular applications expect it to respond to xterm control sequences. I came across this issue (and suspect many others have as well, although it's surprising to me that I didn't see an existing issue for it) because Emacs was hanging for 2 seconds on load whenever being launched inside Windows Terminal due to code in xterm.el (see here) (part of its terminal-capability-determination startup code) that queries the capabilities of xterm using the Secondary Device Attribute control sequence. Other terminals that masquerade as xterm (such as iTerm2, OS X's Terminal, and gnome-terminal) respond to this query.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels May 11, 2020
@zadjii-msft
Copy link
Member

I'm pretty sure we are replying to this message...

bool AdaptDispatch::DeviceAttributes()
{
// See: http://vt100.net/docs/vt100-ug/chapter3.html#DA
return _WriteResponse(L"\x1b[?1;0c");
}

Though, I just tried reading the input with a really simple script and couldn't get any input... Hmm...

@zadjii-msft
Copy link
Member

OH you know what, I see what's happening. We're replying to the Primary Attributes request, but not the other ones.

CSI Ps c  Send Device Attributes (Primary DA).
            Ps = 0  or omitted ⇒  request attributes from terminal.  The
          response depends on the decTerminalID resource setting.
CSI = Ps c
          Send Device Attributes (Tertiary DA).
            Ps = 0  ⇒  report Terminal Unit ID (default), VT400.  XTerm
          uses zeros for the site code and serial number in its DECRPTUI
          response.
CSI > Ps c
          Send Device Attributes (Secondary DA).
            Ps = 0  or omitted ⇒  request the terminal's identification
          code.  The response depends on the decTerminalID resource set-
          ting.  It should apply only to VT220 and up, but xterm extends
          this to VT100.

@zadjii-msft zadjii-msft added Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase labels May 11, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 11, 2020
@zadjii-msft zadjii-msft added this to the Console Backlog milestone May 11, 2020
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 11, 2020
@j4james
Copy link
Collaborator

j4james commented May 11, 2020

These DA reports are on my TODO list, but I've been putting them off because it's difficult to decide what we classify as at this point. We implement sequences from a wide range of terminals, including the VT500 series, but we don't have nearly enough coverage to claim to be something like a VT500. I'm not even sure we could claim to be a VT220 yet. And since the DA2 report doesn't apply to a VT101 (which is what we're currently claiming to be), it didn't make sense to support it yet. That said, we could just go with the XTerm hack that uses 0 to indicate a VT100, and that should at least keep Emacs happy for now.

@zrait
Copy link
Author

zrait commented May 18, 2020

If you can't support the VT220 command set, I do think it makes sense to report 0 as the first digit as that does seem to be what OS X Terminal, iTerm, and gnome-terminal are doing.

@j4james
Copy link
Collaborator

j4james commented Jun 23, 2020

I thought I'd do a quick PR for this now, and immediately got stuck on deciding what to use for the second parameter - what would originally have been the firmware version on a DEC terminal. Some terminal emulators use that parameter to encode their version number (e.g. VTE 0.52.2 will use 5202, and XTerm patch 354 will use 354). Others hardcode a specific XTerm patch number to try and trick applications like VIM into thinking they're XTerm, so they get features like mouse support.

If you want to see the convoluted logic that VIM uses to determine whether certain features can or can't be used, have a look at the code here. As an example of how weird this whole setup is, they have a specific check for PuTTY as reporting version 136, while PuTTY itself only chose 136 to trick VIM into thinking it was XTerm!

So the question is, what do we want to do here? I'd initially thought I could pull something from the conhost file version info. But the system conhost version is just the Window build number, which you may not want to reveal. And the OpenConsole version bundled with WT is something else entirely, which makes it feel kind of pointless. Do we just hardcode something simple like 10 (the DEC equivalent of 1.0)? Or try and pick a number that works well for VIM? I really don't know.

Bottom line is I think this needs input from the core contributors. If you let me know what you'd prefer to use for the version number, I'd be happy to put together the PR.

@ghost ghost added the In-PR This issue has a related PR label Jul 9, 2020
@ghost ghost closed this as completed in #6850 Jul 10, 2020
ghost pushed a commit that referenced this issue Jul 10, 2020
This PR adds support for the `DA2` (Secondary Device Attributes) and
`DA3` (Tertiary Device Attributes) escape sequences, which are standard
VT queries reporting basic information about the terminal.

The _Secondary Device Attributes_ response is made up of a number of
parameters:
1. An identification code, for which I've used 0 to indicate that we
   have the capabilities of a VT100 (using code 0 for this is an XTerm
   convention, since technically DA2 would not have been supported by a
   VT100).
2. A firmware revision level, which some terminal emulators use to
   report their actual version number, but I thought it best we just
   hardcode a value of 10 (the DEC convention for 1.0).
3. Additional hardware options, which tend to be device specific, but
   I've followed the convention of the later DEC terminals using 1 to
   indicate the presence of a PC keyboard.

The _Tertiary Device Attributes_ response was originally used to provide
a unique terminal identification code, and which some terminal emulators
use as a way to identify themselves. However, I think that's information
we'd probably prefer not to reveal, so I've followed the more common
practice of returning all zeros for the ID.

In terms of implementation, the only complication was the need to add an
additional code path in the `OutputStateMachine` to handle the `>` and
`=` intermediates (technically private parameter prefixes) that these
sequences require. I've done this as a single method - rather than one
for each prefix - since I think that makes the code easier to follow.

VALIDATION
----------

I've added output engine tests to make sure the sequences are dispatched
correctly, and adapter tests to confirm that they are returning the
responses we expect. I've also manually confirmed that they pass the
_Test of terminal reports_ in Vttest.

Closes #5836
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jul 10, 2020
@ghost
Copy link

ghost commented Jul 22, 2020

🎉This issue was addressed in #6850, which has now been successfully released as Windows Terminal Preview v1.2.2022.0.:tada:

Handy links:

@DHowett
Copy link
Member

DHowett commented Aug 21, 2020

🎉 As of Windows Insider build 20197, this is also supported in the traditional console.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants