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 repeats character at top left of buffer during every operation #235

Open
melak47 opened this Issue Aug 19, 2018 · 15 comments

Comments

Projects
None yet
6 participants
@melak47

melak47 commented Aug 19, 2018

Windows (Insider) Build is 10.0.17713.1002

I'm trying out the new pseudo terminal support, and capturing the output of a program that simply prints hello, world! to stdout produces the following:

�[2J�[?25l�[39m�[49m�[Hhello, world!��[?25h�[Hh�[Hh�[1;14H�[1;14H

Breaking this down:

0x1B[2J       # clear screen??
0x1B[?25l     # hide cursor
0x1B[39m      # default foreground color
0x1B[49m      # default background color
0x1B[H        # move cursor home??
hello, world! #
0x08          # Backspace key??
0x1B[?25h     # show cursor
0x1B[H        # move cursor home??
h             # print 'h' over the first char??
0x1B[H        #
h             # ...twice??
0x1B[1;14H    # move cursor after printed text
0x1B[1;14H    # ...twice??

Now some of these make sense, but a few I find confusing.
Why clear the screen? (Either the user provided an empty file or a pipe to the PTY, or they intended to collect more than once programs output - at worst the clearing is unnecessary, at worst unwanted)
Why move the cursor to the top left?
Where does the backspace control character come from?
Why is the first character (h) overwritten twice, are erroneously detected changes of the output buffer being rendered as VT sequences here?

@onomatopellan

This comment has been minimized.

onomatopellan commented Aug 19, 2018

Windows (Insider) Build is 10.0.17713.1002

I wouldn't think too much about it then. As Microsoft stated here, API was not truly ready until recent Fast insider builds. (builds 17738 and 17741)

A new Slow Ring build will be released soon and that should contain the complete API.

@zadjii-msft

This comment has been minimized.

Member

zadjii-msft commented Aug 20, 2018

Okay there are a couple things at play here: the Clear Screen at the beginning, and the duplicated "\x1b[Hh"s you're seeing.

The first is an oddity of using the pseudoconsole. Technically, the pseudoconsole is still a console, so it still has it's own buffer that's empty upon initialization. If you were to run your hello world program in a normal console, what you'd see is an empty viewport, with "hello, world!" at the top left.

ConPty needs to re-create that exact state in the terminal that's calling conpty, which is why we start by clearing the screen. Otherwise, imagine a console that's full of output, and calling your hello world application. ConPty doesn't know anything about the state of the terminal that's calling it, so it would still move the cursor to home, then print "hello, world!" over the text that's already in the buffer.

It's weird, yes, but it's unfortunately the only way to maintain compatibility with console applications.

You can however disable this feature by passing the INHERIT_CURSOR flag to CreatePseudoConsole - though be warned, you'll need to make sure that the console is first set to ENABLE_VIRTUAL_TERMINAL_PROCESSING, or your application is running in a terminal capable of responding to a DSR. That'll trick conpty into using the terminal's current cursor position as a starting place for it's own cursor.


The second thing you're seeing, the duplicated "Go home and print 'h'"s, that's a bug. For some reason, something's causing conpty to think that the origin is constantly invalid, so it keeps repainting it. That's a bug I'm already tracking - it showed up only recently, and I don't think it makes the cut for 1809 unfortunately.

@zadjii-msft zadjii-msft self-assigned this Aug 20, 2018

@zadjii-msft zadjii-msft added this to the 19H1 milestone Aug 20, 2018

@kghost

This comment has been minimized.

kghost commented Aug 24, 2018

imagine a console that's full of output, and calling your hello world application. ConPty doesn't know anything about the state of the terminal that's calling it, so it would still move the cursor to home, then print "hello, world!" over the text that's already in the buffer.

This is how Unix/Linux works over half a century, it works perfect.

You can however disable this feature by passing the INHERIT_CURSOR flag to CreatePseudoConsole

Do I need to pass INHERIT_COLOR, INHERIT_UNDERLINE, INHERIT_BOLD, INHERIT_CURSOR_STYLE, INHERIT_WHATEVER_MAY_ADDED_IN_THE_FUTURE? How do I INHERIT_EVERYTHING like I'm in Unix/Linux world ?

@zadjii-msft

This comment has been minimized.

Member

zadjii-msft commented Aug 24, 2018

Unfortunately, it doesn't work perfect on Windows like it does on linux.

In the *nix world, a pty is just a dumb character pipe - it doesn't care where the cursor is, it doesn't even know what a cursor is, that's the job of the terminal.

However, conpty isn't just a dumb pair of character pipes - it's a full windows console, that's attempting to translate the whole of the console API surface into just a stream of characters. So unfortunately conpty has it's own cursor position, independent of the terminal's.

Do I need to pass INHERIT_COLOR, INHERIT_UNDERLINE, INHERIT_BOLD, INHERIT_CURSOR_STYLE, INHERIT_WHATEVER_MAY_ADDED_IN_THE_FUTURE? How do I INHERIT_EVERYTHING like I'm in Unix/Linux world ?

Well, none of INHERIT_COLOR, INHERIT_UNDERLINE, INHERIT_BOLD, INHERIT_CURSOR_STYLE would need to be added - conpty doesn't care about the cursor styles, and it's not possible to query the current color/attribute state of the terminal, so I HIGHLY doubt any of those would ever be added.

@kghost

This comment has been minimized.

kghost commented Aug 26, 2018

So why don't you provide a dumb character pipe API ? All other features can be implemented by the VTE applicatioin.

We need a pty API which provides signal handling, process group, session management. No VTE manipulate at all.

@zadjii-msft

This comment has been minimized.

Member

zadjii-msft commented Aug 27, 2018

Man if only we had a multi-part blog series on why we can't just have a dumb character pipe...

If we just provided a dumb character pipe, then existing console applications wouldn't work with it, because they rely upon the Console API. We want these applications to be compatible with new terminal applications just as a new character-only console application would be.

For many of these console applications, there's no way to be able to reasonably re-write them to exclusively be character-only applications. Case in point - cmd.exe would never be able to be re-written as a character-only application. It makes too much use of the console API surface. That's why there's a pseudo-console, not just a dumb pipe.

Could you elaborate on some of the specific use cases of "signal handling, process group, session management", esp. in relation to pty's? I don't believe any of that is the responsibility of the pty (though I could be mistaken), so I'd imagine there are other Windows-isms that might be able to satisfy those needs in combination with conpty.

@kghost

This comment has been minimized.

kghost commented Aug 28, 2018

You are providing a public API of PTY to developers, nobody cares your vte implementation, your text processing, because communities can and will create our own. In the recent decades, you failed your job, your implementation (cmd.exe) sucks, the whole blog series are just telling how you fail.

Lets look at the new API:

CreatePseudoConsole(size, hInput, hOutput, dwFlags, phPC)
ResizePseudoConsole(hPC, size)
ClosePseudoConsole(hPC)

At first glance, I fount it is amazing, just exactly what I want, but when diving deeper and deeper I found only lots of mines and traps, and nothing else.

To keep compatibility of console applications, why don't you introduce an additional adapt layer, which adapt the raw character pipe to your internal console implementation. You can do whatever you want in the adapt layer, nobody cares. Please keep the public API simple and clean.

signal handling, process group, session management are the foundation of the PTY. Consider there are several tasks (processes) running, one task (aka process group) is running foreground, others are running background, Which process(es) can receive my input ? and when I press CTRL-C, witch process(es) will receive SIGINT ? How to switch active tasks ? It is all controlled by PTY.

@bitcrazed

This comment has been minimized.

Contributor

bitcrazed commented Aug 28, 2018

@kghost We encourage constructive criticism and passionate discussion, but please keep your tone respectful, as per the project’s Code of Conduct.

What we announced in the associated blog post was a Pseudo Console, not a Pseudo Terminal/TTY.

When designing and implementing the ConPTY API, one of our highest priorities was to ensure the correct operation of the many millions of public and private Windows Command-Line apps and scripts in use every day. Our goal was NOT to precisely duplicate *NIX like behavior at the cost of existing Windows Command-Line apps.

As such, Windows Command-Line apps can run attached to a Pseudo Console instance supporting the full Windows Console API, enabling apps to run without requiring any modification. When APIs are called that modify the Console display, the resulting changes are made to ConPTY's internal buffer, and changes are subsequently rendered to the connected Console/Terminal as text/VT.

Unlike *NIX, Windows does not support *NIX signals, and has never supported sessions, process-groups and/or jobs at the Command-Line. In Windows, there is generally only one foreground Command-Line app that receives input and/or emits output at a time per Console. Therefore, there was no compelling need to introduce *NIX-like signals and process-groups etc. to Windows at this time.

However, when operating a Linux instance, for example, running on WSL, or via ssh, keystrokes are sent to the receiving Linux shell/app and should be handled appropriately within the Linux instance. This is how one can run Tmux in Ubuntu on WSL and have your pane-navigation keystrokes, and CTRL+C, etc. handled correctly:

image

Finally, the ConPTY is a new API and a surprisingly sophisticated Pseudo Console infrastructure. There are likely to be a few issues which we’re keen to hear about and will work diligently to fix where we can. Please keep the feedback coming, and do continue to file issues with repro’s.

We look forward to hearing from you all.

@kghost

This comment has been minimized.

kghost commented Aug 29, 2018

You should leverages VTE and PTY, lots of application implement their own VTE instead, and they only needs PTY. There are already lots of VTE implementation, people have rights to choose which one they use, not restrict to the provided one.

Also *nix world have already accumulated vast number of libraries based on PTY interface, if a compatible API are provided, with little or no changes, they can be reused.

From #57 you said:

I do want to make a note, whatever we end up implementing is going to allow existing client applications to continue using the full Win32 console API, and ideally it's going to be without forcing 3rd party terminals from implementing the entire console API surface themselves. We'd love to hide the "black magic", and expose only a sane surface to implement on top of.

But you are still introducing "black magic", this issue is definitely your "black magic". why can't we get a clean PTY api ? At least disable all "black magic" by default.

And with the help of existing VTE libraries, implementing the entire console API surface is pretty easy, given PTY api. Sure someone may prefer provided console API, but CreatePseudoConsole/ResizePseudoConsole/ClosePseudoConsole API is more low level PTY API than console API. So I want to build my own VTE on this API.

This is my opinion:

  1. Leverages VTE and PTY
  2. CreatePseudoConsole/ResizePseudoConsole/ClosePseudoConsole is PTY API
  3. Keep PTY API simple and clean
  4. Provide VTE/CON as a library on top of PTY API.
@zadjii-msft

This comment has been minimized.

Member

zadjii-msft commented Aug 29, 2018

implementing the entire console API surface is pretty easy,

I'm gonna need to stop you there - This is actually straight up impossible. There are a number of Console API's that can't be translated back into VT sequences of any sort. For instance, ReadConsoleOutput. This API lets a client application read an area of the console buffer. In VT, there's no way of doing this, because there is no buffer. In Unix, only the actual terminal emulator has a buffer, the pty is just there to facilitate communication between the terminal and the commandline application.

The last time I took a look at this, these were the console APIs that had no equivalent VT sequence:

AddConsoleAlias
AllocConsole
AttachConsole
FlushConsoleInputBuffer
FreeConsole
GenerateConsoleCtrlEvent
GetConsoleAlias
GetConsoleAliases
GetConsoleAliasesLength
GetConsoleAliasExes
GetConsoleAliasExesLength
GetConsoleCP
GetConsoleCursorInfo
GetConsoleDisplayMode
GetConsoleHistoryInfo
GetConsoleMode
GetConsoleOriginalTitle
GetConsoleOutputCP
GetConsoleProcessList
elements of GetConsoleScreenBufferInfoEx
GetConsoleSelectionInfo
GetConsoleWindow
GetNumberOfConsoleInputEvents
GetNumberOfConsoleMouseButtons
GetStdHandle
HandlerRoutine
PeekConsoleInput
ReadConsole
ReadConsoleInput
ReadConsoleOutput
ReadConsoleOutputAttribute
ReadConsoleOutputCharacter
SetConsoleActiveScreenBuffer
SetConsoleCP
SetConsoleCtrlHandler
SetConsoleHistoryInfo
SetConsoleMode
SetConsoleOutputCP
elements of SetConsoleScreenBufferInfoEx
SetConsoleScreenBufferSize
SetConsoleWindowInfo
SetStdHandle
WriteConsole
WriteConsoleInput
WriteConsoleOutput

why can't we get a clean PTY api ? At least disable all "black magic" by default.

Actually, the conpty API we've created is super simple. You pass in some handles, then you read text and write text two/from those handles.

What we're doing is hiding the "black magic" of the Console API surface from the application calling the conpty API's. In this way, a terminal emulator doesn't need to worry about the fact that for console applications, some api's behave differently with different fonts selected. This isn't the only case where the console API's are weird, and trying to implement all of the quirks of the console API would be a lesson in insanity.

Fortunately, callers of the conpty API's don't need to know anything at all about the console API, and they'll be able to act as terminal emulators for all existing console applications, without those console applications needing to make any changes, to link against new libraries, or change anything.

Also *nix world have already accumulated vast number of libraries based on PTY interface, if a compatible API are provided, with little or no changes, they can be reused.

Hey wadda ya know, the conpty API's are a compatible interface with existing linux terminal emulators. Here's cmd.exe attached to a conpty running attached to gnome-terminal as a head.
image

Or here's some more proof, this is putty (a terminal emulator for windows) modified to use conpty instead of an ssh connection to a linux machine.
image

@kghost

This comment has been minimized.

kghost commented Aug 30, 2018

  1. There are lots of limitations of *nix pty interface. ex, keycode input other than ASCII codes. CONPTY API can support extensions, console API can be implemented with these extensions. But a clean pty compatible API must be provided.

  2. What do you mean hiding the "black magic", I saw some XTerm sequence was injected in to output from nowhere. What if my VT is not XTerm compatible, how can you assume my VT understand XTerm sequence ?

I'm glad you tested the compatibility of common used VTE, but why don't you trust communities ? Provide a clean API and let communities fill the gap. They will do their "open source magic" and complete the job.

@zadjii-msft

This comment has been minimized.

Member

zadjii-msft commented Aug 30, 2018

Re 1: I actually already have a plan in place for a conpty specific extension to enable Console-compatible INPUT_RECORDs via VT, just waiting on having the time to spec and finish it.

re 2: We're trying to hide the "black magic" of the Console API's implementation, rather than expose that and force terminal authors to implement it themselves. This path lets existing linux terminals work with existing console apps with minimal (if any) modifications.

The VT we're synthesizing to render console apps is pretty simple. I'd guess that it's closer to TERM=vt100-265color (if such a termcap existed) than xterm. We're really not emitting anything too extraordinary.

I could imagine in the future enabling other TERM settings for conpty as well, but I do appreciate the simplicity of only having one TERM setting on Windows currently. That way terminal authors know exactly what they're going to get, and don't really need to worry about what the client is trying to do.

I could also imagine a world where we have "passthrough" commandline apps that speak directly to an attached terminal. These would be text-only commandline applications, and then the sequences received by the terminal application would only be what is emitted by the console app. However, these passthrough console apps would have to be authored from scratch, and mixing traditional console apps and passthrough console apps could result in unexpected behavior. It would need a lot of work to get just right, and that's unfortunately not in the cards for the next release. Maybe after that I'll take another look.

I certainly do trust the community to do wonderful things with ConPTY, we're just trying to make their jobs considerably easier. Trying to implement a Console API server with all the existing quirks of the API is insanity, while taking your existing terminal app and hooking it up to conpty is effectively trivial.

@kghost

This comment has been minimized.

kghost commented Aug 31, 2018

these passthrough console apps would have to be authored from scratch, and mixing traditional console apps and passthrough console apps could result in unexpected behavior.

This is why I feel you don't trust communities, leave it to communities, they will fix all unexpected behaviors.

Trying to implement a Console API server with all the existing quirks of the API is insanity, while taking your existing terminal app and hooking it up to conpty is effectively trivial.

From my point of view, hooking up to pure pty is much more easier. Check wslbridge, it exposes a pure pty interface, and there are already several terminal implementations build on it. How could something which has already been easily achieved be insanity ?

@zadjii-msft

This comment has been minimized.

Member

zadjii-msft commented Sep 4, 2018

Ah but see, wslbridge, winpty, etc are trying to be pseudoterminals on Windows. That's a much easier task then being a pseudoconsole. wslbridge and things like mintty are really useful at running linux-like commandline applications that only read and write text. That's why wslbridge works so well with WSL - any apps you're running in WSL are just reading and writing text. They aren't using the Console API, and they don't emulate the Console behavior. They simply act as Terminal emulators, not Console emulators.

In this sense, WSLBridge exposes a pty interface for applications that can use a TTY interface.

What WSLBridge doesn't do is expose a pty interface for the Console interface. That was our goal with conpty.

If you really want to write a passthrough style application, that only reads and writes text, I'm sure it'll work great in wslbridge/winpty without any extra work, and you can surely write a terminal emulator on top of those interfaces today. However, if you want to be able to write a terminal emulator that can be attached to console applications, using the full console API, you wouldn't be able to do that with wslbridge alone.

This was referenced Sep 24, 2018

@DHowett-MSFT DHowett-MSFT changed the title from ConPTY unexpected output to ConPTY repeats character at top left of buffer during every operation Oct 3, 2018

@miniksa

This comment has been minimized.

Member

miniksa commented Oct 22, 2018

For reasons that are hard to explain fully... fixing #279 also contains the fix for this.

Well... it's not that hard to explain... While I was fixing the code for optimizing scrolling, I needed to improve our class for manipulating rectangles and coordinates. And while I was improving that class and adding unit tests, I found an error-prone constructor scattered throughout the code. And then I removed that constructor and lo-and-behold, one of the error-prone uses of the constructor was the ConPTY renderer and it was using it in an erroneous way. Now it's not.

The ConPTY renderer was using the rectangle inclusive of the coordinate 0,0 as its default invalidation area when it ended paint because @zadjii-msft thought he was setting it to an empty rectangle but aforementioned erroneous constructor made it very easy to accidentally set the rectangle encompassing 0,0 instead. Oops.

So the problem should be solved in 3-4 weeks in Insiders builds and I've made it harder for us to make this mistake again in the future.

@miniksa miniksa added the fix-inbound label Oct 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment