Skip to content

Commit

Permalink
Enable echo for runnerv2 and in terminal mode
Browse files Browse the repository at this point in the history
  • Loading branch information
adambabik committed May 21, 2024
1 parent 5ced265 commit 63a836a
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 18 deletions.
20 changes: 13 additions & 7 deletions internal/command/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type Command interface {
}

type baseCommand interface {
IsEchoEnabled() bool
Env() []string
ProgramConfig() *ProgramConfig
ProgramPath() (string, []string, error)
Expand All @@ -36,17 +37,22 @@ type internalCommand interface {
}

type base struct {
cfg *ProgramConfig
kernel Kernel
session *Session
stdin io.Reader
stdinWriter io.Writer
stdout io.Writer
stderr io.Writer
cfg *ProgramConfig
isEchoEnabled bool
kernel Kernel
session *Session
stdin io.Reader
stdinWriter io.Writer
stdout io.Writer
stderr io.Writer
}

var _ internalCommand = (*base)(nil)

func (c *base) IsEchoEnabled() bool {
return c.isEchoEnabled
}

func (c *base) Interactive() bool {
return c.cfg.Interactive
}
Expand Down
6 changes: 4 additions & 2 deletions internal/command/command_virtual.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ func (c *virtualCommand) Start(ctx context.Context) (err error) {
return errors.WithStack(err)
}

if err := disableEcho(c.tty.Fd()); err != nil {
return err
if !c.IsEchoEnabled() {
if err := disableEcho(c.tty.Fd()); err != nil {
return err
}
}

program, args, err := c.ProgramPath()
Expand Down
21 changes: 14 additions & 7 deletions internal/command/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import (
)

type Options struct {
// EnableEcho enables the echo when typing in the terminal.
// It's respected only by interactive commands, i.e. composed
// with [virtualCommand].
EnableEcho bool
Session *Session
StdinWriter io.Writer
Stdin io.Reader
Expand Down Expand Up @@ -63,6 +67,8 @@ func (f *commandFactory) Build(cfg *ProgramConfig, opts Options) Command {
case runnerv2alpha1.CommandMode_COMMAND_MODE_TERMINAL:
// For terminal commands, we always want them to be interactive.
cfg.Interactive = true
// And echo typed characters.
opts.EnableEcho = true

return &terminalCommand{
internalCommand: f.buildVirtual(f.buildBase(cfg, opts)),
Expand All @@ -80,13 +86,14 @@ func (f *commandFactory) Build(cfg *ProgramConfig, opts Options) Command {

func (f *commandFactory) buildBase(cfg *ProgramConfig, opts Options) *base {
return &base{
cfg: cfg,
kernel: f.kernel,
session: opts.Session,
stdin: opts.Stdin,
stdinWriter: opts.StdinWriter,
stdout: opts.Stdout,
stderr: opts.Stderr,
cfg: cfg,
isEchoEnabled: opts.EnableEcho,
kernel: f.kernel,
session: opts.Session,
stdin: opts.Stdin,
stdinWriter: opts.StdinWriter,
stdout: opts.Stdout,
stderr: opts.Stderr,
}
}

Expand Down
1 change: 1 addition & 0 deletions internal/runnerv2service/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func newExecution(
stderr := rbuffer.NewRingBuffer(ringBufferSize)

cmdOptions := command.Options{
EnableEcho: true,
Session: session,
StdinWriter: stdinWriter,
Stdin: stdin,
Expand Down
8 changes: 6 additions & 2 deletions internal/runnerv2service/service_execute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ func TestRunnerServiceServerExecute_Configs(t *testing.T) {
Interactive: true,
},
inputData: []byte("Frank\n"),
expectedOutput: "My name is Frank\r\n",
expectedOutput: "Frank\r\nMy name is Frank\r\n",
},
{
name: "Python",
Expand Down Expand Up @@ -592,7 +592,11 @@ func TestRunnerServiceServerExecute_WithInput(t *testing.T) {
result := <-execResult

assert.NoError(t, result.Err)
assert.Equal(t, "A\r\nB\r\nC\r\nD\r\n", string(result.Stdout))
expected := "a\r\nb\r\nc\r\nd\r\nA\r\nB\r\nC\r\nD\r\n"
// On macOS, the ctrl+d (EOT) character followed by two backspaces is collected.
// On Linux, in the CI, this does not occur.
got := string(bytes.ReplaceAll(result.Stdout, []byte("^D\b\b"), nil))
assert.Equal(t, expected, got)
assert.EqualValues(t, 0, result.ExitCode)
})

Expand Down

0 comments on commit 63a836a

Please sign in to comment.