Interested in seeing this happen. Let me know if I can help in any way...

@jeffreystoke I apologize, I still have this in my todo list, will get to it when I can.

@mattcanty This is quite a large change and I haven't find time yet to look into it. Need to test it, make sure it doesn't break existing codebases, doesn't force a large dependency tree, etc.

Ideally, we'd need some automated testing... Checking for various exec modes, daemons, foreground, etc, and ensure that it works in all OS/Arch while staying backward compatible

I managed to try on windows, however, I couldn't get anything to work. Would you have a sample code that would demonstrate how to use this on windows?

All my tests ended up with an error from pty.Start:

CreatePseudoConsole: The operation completed successfully.

A bit strange to have "success" to be an error, but it might be because I do something wrong.

I tried using the lib directly, and I also tried the lib your linked to (arhat.dev/pkg/exechelper), but without any luck.

Hi @creack and @jeffreystoke,

Glad to see some action here. It seems this is one of many libraries looking to make use of this new tool. You may be interested in some work done by @marcomorain in his repo go-conpty. I too have been searching for a Windows PTY solution but have ended up arriving at similar issues.

@creack #109 (comment)

Did you run as system admin? If not, can you try again with admin privileges to see whether it's a security issue?

As noted before, I'm not a windows expert, you may have to seek help from others, good luck :)

Thanks for working on this. I tried the following on Windows, but got an error:

if err := pty.InheritSize(os.Stdin, ptmx); err != nil {
  log.Printf("error resizing pty: %s", err)
}

produced "error resizing pty: The handle is invalid."

Would you mind adding an example for windows in the readme? Once merge, this PR will be gone and the links provided in the description forgotten.

nit: if you could push commits, it would make review easier. Keeping everything in one commit with force push is neat and tidy but makes it difficult to see what changed and requires a full re-review.
We can squash at the end while merging, resulting in a single commit.

Would you mind adding an example for windows in the readme? Once merge, this PR will be gone and the links provided in the description forgotten.

The usage is the same with other platforms', so I don't think it's necessary to create a special windows example, but we can note requirements for windows platform, wdyt?

Requirements: https://docs.microsoft.com/en-us/windows/console/createpseudoconsole#requirements

nit: if you could push commits, it would make review easier. Keeping everything in one commit with force push is neat and tidy but makes it difficult to see what changed and requires a full re-review.
We can squash at the end while merging, resulting in a single commit.

Ok, I will do that for following changes.

@blasten The handle invalid error means the provided os.Stdin is not a ConPty handle, we have to add support for old windows console api to make it working, I guess that would happen in a new pull request?

Sorry, didn't get time to dig into this yet. Something came to my attention though, no context, no detail but saw this:

leokhoa@4fa3b1e

As the lib would now panic if the DLL loading fails, we'll need to figure out want's going on, and whether fix the issue or make sure importing the lib doesn't break unrelated applications if a dll is missing.

@creack No idea why that would panic, but never mind, I have just pushed the latest implementation using LazyDLL and LazyProc, it will never panic on dll loading/proc finding failures as noted in my commit message.

fyi, go1.17 disabled //go:linkname support for private struct member, so this pull request will probably not be merged

ref: golang/go#46777 (comment)

Made it working for go1.17 according to the issue referenced above (also works with go1.16, probably will work for earlier go toolchain)

Is there anything that can be done with "The operation completed successfully." being reported as error when calling CreatePseudoConsole or Resize?

@jeffreystoke I made an attempt to fix the issue I was talking about and seeing if I could fix render bug. I do not work with Windows a lot. I could just been luck when I was testing my code, but it seem to fixed the issue. My copy of your fork: https://github.com/photostorm/pty

@photostorm me neither.

re: your fix

Good point, I completely misused the COORD, would you like to merge it on your own or something else?

pty-patch.txt

I created a patch file for you.

Is something still missing here so that this can be merged into main?

@creack are you available to take a glance to see if this matches and fixes the requested changes?

Apologies for a non-helpful 👍 comment, but I've been eagerly watching this PR since it was opened, and it would be a great xmas present to see it merged.

I still haven't had the time to dig into this, I am sorry.

If someone could come up with an easy way to test it, it would be quite helpful as it is not clear to me in which scenario it is supposed to be working.

What we're hoping to use this for is a Go based SSH server where the terminal is provided by this package. At present the server works great on UNIX platforms, using this package, but has to fall back to plain stdout for Windows.

We're using this high level SSH server wrapper for the low level Go SSH package: https://github.com/gliderlabs/ssh

Just tried the pty example from that repository, it doesn't even compile, which is quite bad as even if it is not supported, it shouldn't break compilation.
I'll push a fix for that asap and will keep digging. Still not sure what/how to test though. Once I run the pty example, how do I check that this PR works as expected? What am I supposed to see before/after?

You should have a working terminal, honestly that's it. Like pressing "up", ctrl-C, stuff like that should work. Previously you literally couldn't make a pty with your package at all on Windows, at the level of the code (intentionally) not compiling there.

I just cloned the branch, it doesn't compile (go1.18)
Getting:

# github.com/creack/pty
.\cmd_windows.go:24:3: //go:linkname must refer to declared function or variable

Having never done Windows (or indeed any C/C++) bindings in Go, I couldn't offer better suggestions than you'd find on Google. Does this need to be compiled on a Windows machine?

At any rate, it seems like Windows would have to be added to the CI pipeline for this project in order to keep it building on Windows at very least.

Hi,

I also tried this patch a few months ago. It compiled back then but had several issues with window resizing and other things I don't rember well. I added this feature externally to the library using conpty, unfortunately the code was part of a proprietary project and I cannot share it. I suggest to try conpty to the ones interested in this feature

Can't build with go1.18.1:

../go/pkg/mod/github.com/donorp/pty@v1.1.12-0.20211004111936-294eccab62ed/cmd_windows.go:24:3: //go:linkname must refer to declared function or variable

I have opened a new pull request for ConPTY if everyone would like to try it.

#155