vhs icon indicating copy to clipboard operation
vhs copied to clipboard

calling `vhs record` prints an error message instead of recording a new tape

Open Jonathan-Zollinger opened this issue 2 years ago • 5 comments

Brief Overview

calling vhs's record prints an error message and doesn't produce a vhs tape

Expected Behavior

Calling vhs record creates a vhs tape and records the user's terminal actions in the vhs tape.

Actual Behavior

Calling vhs record prints out unsupported and stops. (see gif below)

Environment

OS Terminal Terminal Emulator VHS Version
Windows 11 Home
Build 22621.1344
PowerShell 7.3.2 Windows Terminal
as Administrator
v0.3.0 (7cb57d1)

installed via scoop. currently upgraded to latest available build scoop offers

I'm happy to help debug however I can or provide any other information. logging this bug per @bashbunni in discord

Jonathan-Zollinger avatar Mar 08 '23 18:03 Jonathan-Zollinger

This is an unfortunate error from github.com/creack/pty, which doesn't seem to support Windows:

https://github.com/charmbracelet/vhs/blob/1d202a46b770170420a3c29983d8cd5b0d43d0d6/record.go#L72-L75

https://github.com/creack/pty/blob/0d412c9fbeb14954aa65830dcfdb84005cd0cd96/doc.go#L11

picatz avatar Mar 13 '23 00:03 picatz

Looks like there's an open PR for Windows support for Pty. If this is time sensitive, you might be better off building VHS from source and adjusting the dependency to use the upstream that supports Windows. Hopefully that all gets resolved on our upstream soon, but as it involves breaking changes to their API, I can understand why it is taking a while to consider.

https://github.com/creack/pty/pull/155

I'll talk to the team about what can be done on our end to support Windows users without breaking our project 😅

bashbunni avatar Mar 24 '23 01:03 bashbunni

sounds great! thanks !

Jonathan-Zollinger avatar Mar 24 '23 17:03 Jonathan-Zollinger

Would it make sense to add a warning on Windows to this section? It's a bit jarring and has turned my off to using VHS in the past.

terminal, err := pty.Start(command) 
 if err != nil { 
        if runtime.GOOS == "windows" {
                log.Warnf("Windows dependency pty : %q\n", err)
        }
     	return err 
 } 

With the progress on this issue (about an update a month) it doesn't seem like this will be quickly resolved. The last comment is a reference to a PR on the Go Gerrit to add support for Window's PsudoConsole Syscall. No response on the parent issue regarding this, plus depending on if we're in a Go feature freeze we might have a few months to wait regardless if this new SysCall is Pty's solution not counting the time for Pty to finish their tests. Current tests are still failing on Linux according to their CI.

tristanisham avatar Apr 11 '24 22:04 tristanisham

@tristanisham that's a good idea, I think it would be worth adding

bashbunni avatar Apr 22 '24 20:04 bashbunni