-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
ConPTY does not pass Device Control Strings (DCS) to 3rd-party terminals #17313
Comments
Hi I'm an AI powered bot that finds similar issues based off the issue title. Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one and thumbs upping the other issue to help us prioritize it. Thank you! Closed similar issues:
|
Did you use the system ConPTY API? Due to the lengthy release cycle of Windows that API is usually quite outdated. We're currently experimenting with shipping ConPTY as a nupkg. You can find a build here: https://terminalbuilds.blob.core.windows.net/builds/Microsoft.Windows.Console.ConPTY.1.19.240130002.nupkg To be fair, I haven't tried your repro, so my suggestion may be pointless. 🙈 However, even if that nupkg doesn't fix the issue, you'd still need to use a bundled OpenConsole.exe version to receive an updated version later on. But the good news is that I'm currently working on replacing the complex ConPTY setup we have. Afterwards it'll pass VT text from your shell, etc., 1:1 directly to the terminal. The bad news is that I am currently working on it. If the above .nupkg doesn't work, and I'm not sure if you can share your timeline of shipping Warp on Windows publicly, but if it's still a while out, you could consider waiting for me to finish my work on that. |
Hi @lhecker thanks for taking the time to respond! I'm another engineer at Warp, working with @acarl005.
We followed the instructions outlined in the Mastodon thread you linked but unfortunately using the newer ConPTY build does not solve the issue. Specifically, I still see that DCS strings are not getting sent from ConPTY and that the custom OSCs we use as an alternative are sent out of order.
Noted! We'll ship with these newer bundles instead of relying on the system API.
This is exactly what we need, looking forward to this releasing! 🥳 Many of the features we use would depend on this particular part of the PTY since we rely on it to implement many of our key features, like blocks. We can't meaningfully test most of the core functionality of Warp without this. Do you have a rough timeline (weeks/months/quarters) of when this might get shipped? Our intention is not to rush you but just to plan around it! |
I think a month is a fair estimate for this. I don't think it'll take multiple months. |
@lhecker Awesome! We're happy to hear someone is working on this. May I ask about order preservation in your planned implementation? As for Warp's requirements, we use DCS for things like delimiting our "blocks" and so ordering is an important for our case, e.g. if a process emits something like... echo "some output `eP <some data for terminal> `e\ some other output" ...then we depend on |
@lhecker We'd also love to try out the prototype you are building, if you can easily share it! We can try it from a remote branch, a specific build, or anything else that's convenient. |
If you have
I'm not at a point yet where you could rely on it, but my WIP branch is here: https://github.com/microsoft/terminal/tree/dev/lhecker/goodbye-vtengine The design spec is here: https://github.com/microsoft/terminal/blob/dev/lhecker/goodbye-vtengine/doc/specs/%2313000%20-%20In-process%20ConPTY.md |
@lhecker It sounds like this is essentially "passthrough mode", is that correct? Please let us know if Warp engineers can help with this migration in any way. We would likely be able to commit at least one engineer to this effort to help get it launched. |
@alokedesai Not in the original sense. (Some more context for those who may not be super familiar with ConPTY's architecture:) ConPTY parses any VT output of any application and stores the results in a local buffer, so that later calls to the Passthrough mode used to be the idea that we could stop parsing the VT output of any application that promises to only ever use VT and not call anything except for My upcoming proposal on the other hand is to always parse the output, despite the cost. I've improved the performance of our text buffer by >10x over the last 3 years so it's not as much of an issue anymore. My above branch for instance gets ~100MB/s with |
Gotcha, that makes sense. Agreed that an approach that still supports the traditional Console APIs is strictly better
I see. So TLDR; whereas previously the |
@lhecker Really excited to see this happen, if Warp engineers can help out with this in anyway please let us know. This seems like a massive rearchitecture of ConPTY and we would be able to contribute headcount if it would accelerate the process of launching this. |
As a tl;dr, absolutely!
Thanks! I'll definitely keep this in mind. 🙂 For now, however, it needs to be approved by the team and probably also run by our division architects, because all 4 steps in the spec together are a huge change that will likely take >1 year to complete. If you're massively blocked by this issue, and if you have dev time to spare, you could consider hotfixing this issue yourself. I believe what you need is something like this: terminal/src/terminal/adapter/adaptDispatch.cpp Lines 3762 to 3767 in baba406
If that's correct, then you then need to call out to terminal/src/terminal/parser/OutputStateMachineEngine.cpp Lines 717 to 756 in baba406
and do the same |
Great thank you, we'll try that out! Will that suggested hotfix also solve the ordering problem? (My assumption is no but just confirming). And to clarify, is sending DCS to the terminal + fixing the ordering problems still something you expect to take 1 month? I assume productionizing the server and shipping it in windows are what would cause this to take 1 year+? |
Just FYI, I made a conscious decision not to pass through unrecognised If you follow other terminal bug trackers, you can see the kind of problems this causes. And app devs are affected by the issue as well, because instead of a particular sequence simply not being supported on Windows, it appears to work, but has subtle bugs. And they waste a whole lot of time trying to understand and work around those bugs, when it's really not something they can fix. So if you want Another option to consider would be adding a pass-through wrapper sequence like some multiplexers support (e.g. see the tmux FAQ). After all, the current implementation of conpty is essentially a kind of multiplexer. But really I think the best bet is to wait for lhecker's new architecture. If it turns out that isn't going to work, we can always revisit other options later. |
@lhecker What's the simplest way to build your prototype from dev/lhecker/goodbye-vtengine? I'm assuming that all I need is to successfully build When I run the PowerShell function I am able to successfully get the Happy to fix some of the smaller errors like type conversions or an incorrect number of arguments, but I figured I should check on what projects I actually need. Here's the Visual Studio |
@j4james By "getting If so, very much agreed. We tried |
It's a different branch but I did a quick screen recording for how I do it: kjhpAjAZr36Fm3aK.mp4
When it comes to OpenConsole and conpty.dll: I just pushed a commit that fixes the build error. Instead of |
@alokedesai That's one issue. Another problem is that the position of the cursor when you receive the sequence won't necessarily be the position of the cursor when it was sent, so you can't depend on that. And if you change the cursor position yourself in response to a passed-through sequence, that has the potential to misposition any subsequent output. Or if you produce any output yourself in response to a passed-through sequence, that will potentially be overwritten by the conpty renderer at a later stage. In short, there are very few scenarios that can reliably work with a passed-through sequence if conhost isn't specifically designed to handle that sequence. As a rule of thumb, if you can't make it work in tmux, it's unlikely to work through conpty. |
@j4james @lhecker Thanks for the additional info. Warp blocks work by sending a custom DCS from shell --> terminal when a command starts and finishes executing (see a blog post about this here). Whenever a command finishes, we create a new grid so that contents from each command are fully isolated from each other. Given that background, I don't think a fork of ConPTY that adds support for a custom OSC (or DCS) would fix the issues we're seeing. This is because ConPTY would have to actually update its own internal data model to reflect the fact that Warp is going to create a new grid, which I think is impossible to do perfectly (but I may be overestimating the complexity of this). Assuming my understanding is correct (I'd love confirmation of that / whether there's an obvious solution I'm missing) I believe we have to wait for changes outlined in step 1 of that design doc to land. Is ~1 month still accurate? Happy to Zoom about this if easier, I believe we chatted with some of your coworkers in the past (@DHowett and Carlos Zamora). |
Yeah, by starting a new "block" we're basically starting a new grid, which sets the cursor position back to (0, 0). This puts us out of sync with the console output buffer unfortunately. |
Hi @lhecker, following up here one more time. I noticed that the original design spec you posted no longer exists. Is this something you're still working on? |
(Replied via e-mail!) |
(for everyone not on the mail thread (me too), the spec is in #17387) |
Yep, exactly, the spec moved over to a PR now, while I continue to implement the new ConPTY. I'm currently working on a new "cooked read" implementation (= Windows' readline-like implementation for |
Hi @lhecker, thanks for all of your hard work on this. Really exciting to see some of the PRs go by. I'm trying to better understand the end-state after this work is complete: under what circumstances will ConHost rely on the output buffer and try to read-back what's changed to send VT sequences to the terminal? I originally was under the impression that this change would completely remove the need for ConHost buffer, but I realize that's not possible because it it would break console functions that require reading the buffer (such as This has an impact on Warp because our data model isn't a simple grid, so I'm trying to think through if the new architecture would actually fully solve our requirements. |
ConPTY, the way it works right now (where it emits a pure stream of VT), will continue to exist forever, because applications like sshd don't really have a buffer to read from either. The new architecture proposed in the spec is a low-level layer on which ConPTY is built on top of. If you give it the ability to read back various info, it gives you a practically zero overhead access to the Windows console API and complete freedom to determine what constitutes a grapheme cluster, etc. In other words, you really don't need to use it, unless you want to or otherwise have a specific need for it. Although I'm somewhat surprised you mentioned The 2nd hardest thing in the low-level API is likely implementing the read-back functionality for the buffer contents. But I think it's feasible to implement for most, because the "grid" for reading is conceptually identical to the grid for writing VT. I know that Warp divides output into "Blocks" but I'm assuming it still has support for CUP sequences and the like, so it has support for a VT viewport and its grid too, right? Since reading from the buffer uses the same grid, I think it should generally be possible to implement with your buffer architecture as well. The hardest API is likely the fact that the console API allows the client to create arbitrarily many off-screen text buffers whose size differs from the window size ( All that said, and as I said above, you really don't need to use it if you have no need for it. |
The related PR is now more or less done, although I expect it to receive smaller fixes in the future. If you'd like to experiment with it, you can download our Canary build here: #16121 It currently includes my PR as a test. Theoretically it should be possible for you to replace your current OpenConsole.exe with the one from the Canary build and it should just work (you can find OpenConsole.exe inside the 3 portable ZIPs). I recommend using overlapped IO for optimal performance. If you find any bugs or issues, please let me know! |
Apologies for the delay in response here. Thanks for the detailed response; we'll test this out locally and file any issues if we see them! |
Out of curiosity, do you have a sense of when that change would hit preview? Is this the issue I should be following? #17643 |
We'll likely release this in preview at the end of this month, but there's no fixed date yet. I don't think I'll be able to address every point in #17643 any time soon, so it's not going to be closed by then. It's enough to keep following this issue, because our bot will write here whenever it gets released. 🙂 |
Windows Terminal version
1.19.11213.0
Windows build number
10.0.22631.0
Other Software
No response
Steps to reproduce
We're thrilled by the release and development of ConPTY. We're hoping it can support our usage of Device Control Strings.
From any 3rd-party terminal that wishes to "speak ANSI" using ConPTY, run any command that outputs a DCS escape sequence. (I'm using a locally built alacritty)
Alacritty does not receive the DCS.
Expected Behavior
I expect DCS escape sequences to be output by ConPTY so that 3rd-party terminals may use this to implement more advanced features.
Context
I work for Warp and we'd like to bring Warp Terminal to Windows. Warp depends heavily on DCS strings for its key features.
Alternatives Considered
We tried utilizing a custom OSC sequence instead. We notice that unhandled OSC gets flushed to the terminal. Although with this method Warp is able to receive these data as we'd like, the ordering is not preserved, i.e. if the shell outputs an OSC interleaved with text, when it is read out of ConPTY by the terminal, the OSC is not received in the same order in which it was produced. Warp does depend on that ordering, which is an invariant on UNIX PTYs.
It would be nice if DCS had similar logic where
_pfnFlushToTerminal
was called like it is for OSC, but where ordering is preserved.Actual Behavior
Only DCS escape sequences that Windows Terminal recognizes are being dispatched. As we understand it, ConPTY is an interface intended to be used by 3rd-party terminals for optimal compatibility. Therefore, it makes sense to expand support for arbitrary DCS sequences that other terminals might depend on.
The text was updated successfully, but these errors were encountered: