stateful / runme

DevOps Notebooks Built with Markdown
https://runme.dev
Apache License 2.0
1.08k stars 33 forks source link

Interactive inputs do not produce "echo" #566

Closed sourishkrout closed 3 months ago

sourishkrout commented 4 months ago

Simple example is running read. Unclear what needs to toggle whether or not echoing is enabled. Might have to do some digging in Runnerv1.

config:{program_name:"/bin/zsh" directory:"/Users/sourishkrout/Projects/stateful/oss/vscode-runme/examples" env:"RUNME_ID=01HX7QEZTB94V7A28K766WNRG3" env:"TERM=xterm-256color" commands:{items:"read" items:""} interactive:true mode:COMMAND_MODE_INLINE known_id:"01HX7QEZTB94V7A28K766WNRG3"} winsize:{rows:10 cols:177} session_id:"01HX7QGJ4VNB9YYEC3NCHD908M" store_stdout_in_env:true

https://github.com/stateful/runme/blob/e163a3a9a20284457c8c518b753b62257e66a800/internal/command/command_virtual.go#L98-L100

adambabik commented 4 months ago

It's disabled by default: https://github.com/stateful/runme/blob/e163a3a9a20284457c8c518b753b62257e66a800/internal/command/command_virtual.go#L98-L100

Should it be enabled or configurable?

sourishkrout commented 4 months ago

Should it be enabled or configurable?

I wish I could remember the logic because it wasn't just always on or always off. Do you see anything in the runnerv1 implementation that's revelatory?

@adambabik when you search for DisableEcho it appears that the echo is enabled when a TTY is attached which is always true for a VirtualCommand, no?

adambabik commented 4 months ago

I wish I could remember the logic because it wasn't just always on or always off. Do you see anything in the runnerv1 implementation that's revelatory?

In the runnerv1 service, echo is always enabled. When using the CLI, executing a program which is a file or shell, and the block is marked as interactive, which is a default, then echo is disabled. I am not sure if it was well thought out, but the fact is that in runnerv2 echo is always disabled. Should I flip it to stay compatible?

@adambabik when you search for DisableEcho it appears that the echo is enabled when a TTY is attached which is always true for a VirtualCommand, no?

This is true and VirtualCommand is used when the block is interactive, which is a default.

sourishkrout commented 4 months ago

@sourishkrout yes, let's go ahead and flip it please. Being "incompatible" otherwise means the integration with the notebook terminal stops working as expected.

adambabik commented 3 months ago

@sourishkrout I merged #576. Let me know if there is anything else that should be done as a part of this ticket.

sourishkrout commented 3 months ago

@sourishkrout I merged #576. Let me know if there is anything else that should be done as a part of this ticket.

This looks good now 👍 . Thank you!