Fix CloseReader

coder · kylecarbs · Mar 22, 2022 · Mar 10, 2022 · Mar 10, 2022 · Mar 11, 2022
commit 749d1ac64b7d78798b8c0889573b6710679c93c5
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -43,6 +43,7 @@
     "hclsyntax",
     "httpmw",
     "idtoken",
+    "Iflag",
     "incpatch",
     "isatty",
     "Jobf",
@@ -69,7 +70,9 @@
     "retrier",
     "sdkproto",
     "stretchr",
+    "TCGETS",
     "tcpip",
+    "TCSETS",
     "tfexec",
     "tfstate",
     "trimprefix",

diff --git a/cli/cliui/cliui.go b/cli/cliui/cliui.go
@@ -1,8 +1,13 @@
 package cliui
 
 import (
+	"context"
+
+	tea "github.com/charmbracelet/bubbletea"
 	"github.com/charmbracelet/charm/ui/common"
 	"github.com/charmbracelet/lipgloss"
+	"github.com/coder/coder/pty"
+	"github.com/spf13/cobra"
 	"golang.org/x/xerrors"
 )
 
@@ -44,3 +49,19 @@ var Styles = struct {
 	Logo:          defaultStyles.Logo.SetString("Coder"),
 	Wrap:          defaultStyles.Wrap,
 }
+
+func startProgram(cmd *cobra.Command, model tea.Model) error {
+	readWriter := cmd.InOrStdin().(pty.ReadWriter)
+	cancelReader, err := readWriter.CancelReader()
+	if err != nil {
+		return err
+	}
+	ctx, cancelFunc := context.WithCancel(cmd.Context())
+	defer cancelFunc()
+	program := tea.NewProgram(model, tea.WithInput(cancelReader), tea.WithOutput(cmd.OutOrStdout()))
+	go func() {
+		<-ctx.Done()
+		program.Quit()
+	}()
+	return program.Start()
+}
diff --git a/cli/cliui/list.go b/cli/cliui/list.go
@@ -38,8 +38,8 @@ func List(cmd *cobra.Command, opts ListOptions) (string, error) {
 		opts:  opts,
 		model: model,
 	}
-	program := tea.NewProgram(listModel, tea.WithInput(cmd.InOrStdin()), tea.WithOutput(cmd.OutOrStdout()))
-	err := program.Start()
+
+	err := startProgram(cmd, listModel)
 	if err != nil {
 		return "", err
 	}

diff --git a/cli/cliui/list_test.go b/cli/cliui/list_test.go
@@ -0,0 +1,54 @@
+package cliui_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/coder/coder/cli/cliui"
+	"github.com/coder/coder/pty/ptytest"
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/require"
+)
+
+func TestList(t *testing.T) {
+	t.Parallel()
+	t.Run("Select", func(t *testing.T) {
+		t.Parallel()
+		ptty := ptytest.New(t)
+		msgChan := make(chan string)
+		go func() {
+			resp, err := list(ptty, cliui.ListOptions{
+				Title: "Example",
+				Items: []cliui.ListItem{{
+					ID:          "some",
+					Title:       "Item",
+					Description: "Description",
+				}, {
+					ID:          "another",
+					Title:       "Another",
+					Description: "Another one!",
+				}},
+			})
+			require.NoError(t, err)
+			msgChan <- resp
+		}()
+		ptty.ExpectMatch("Description")
+		ptty.Write(ptytest.KeyDown)
+		ptty.WriteEnter()
+		require.Equal(t, "another", <-msgChan)
+	})
+}
+
+func list(ptty *ptytest.PTY, opts cliui.ListOptions) (string, error) {
+	value := ""
+	cmd := &cobra.Command{
+		RunE: func(cmd *cobra.Command, args []string) error {
+			var err error
+			value, err = cliui.List(cmd, opts)
+			return err
+		},
+	}
+	cmd.SetOutput(ptty.Output())
+	cmd.SetIn(ptty.Input())
+	return value, cmd.ExecuteContext(context.Background())
+}
diff --git a/cli/cliui/prompt.go b/cli/cliui/prompt.go
@@ -1,7 +1,6 @@
 package cliui
 
 import (
-	"context"
 	"strings"
 
 	"github.com/charmbracelet/bubbles/textinput"
@@ -31,14 +30,7 @@ func Prompt(cmd *cobra.Command, opts PromptOptions) (string, error) {
 	input.model.EchoMode = opts.EchoMode
 	input.model.Focus()
 
-	ctx, cancelFunc := context.WithCancel(cmd.Context())
-	defer cancelFunc()
-	program := tea.NewProgram(input, tea.WithInput(cmd.InOrStdin()), tea.WithOutput(cmd.OutOrStdout()))
-	go func() {
-		<-ctx.Done()
-		program.Quit()
-	}()
-	err := program.Start()
+	err := startProgram(cmd, input)
 	if err != nil {
 		return "", err
 	}

diff --git a/cli/cliui/prompt_test.go b/cli/cliui/prompt_test.go
@@ -62,6 +62,6 @@ func prompt(ptty *ptytest.PTY, opts cliui.PromptOptions) (string, error) {
 		},
 	}
 	cmd.SetOutput(ptty.Output())
-	cmd.SetIn(ptty.Input().Reader)
+	cmd.SetIn(ptty.Input())
 	return value, cmd.ExecuteContext(context.Background())
 }
diff --git a/cli/login_test.go b/cli/login_test.go
@@ -34,9 +34,9 @@ func TestLogin(t *testing.T) {
 		// accurately detect Windows ptys when they are not attached to a process:
 		// https://github.com/mattn/go-isatty/issues/59
 		doneChan := make(chan struct{})
-		root, _ := clitest.New(t, "login", client.URL.String(), "--force-tty")
+		root, _ := clitest.New(t, "login", client.URL.String())
 		pty := ptytest.New(t)
-		root.SetIn(pty.Input().Reader)
+		root.SetIn(pty.Input())
 		root.SetOut(pty.Output())
 		go func() {
 			defer close(doneChan)
@@ -66,9 +66,9 @@ func TestLogin(t *testing.T) {
 		coderdtest.CreateFirstUser(t, client)
 
 		doneChan := make(chan struct{})
-		root, _ := clitest.New(t, "login", client.URL.String(), "--force-tty", "--no-open")
+		root, _ := clitest.New(t, "login", client.URL.String(), "--no-open")
 		pty := ptytest.New(t)
-		root.SetIn(pty.Input().Reader)
+		root.SetIn(pty.Input())
 		root.SetOut(pty.Output())
 		go func() {
 			defer close(doneChan)
@@ -90,9 +90,9 @@ func TestLogin(t *testing.T) {
 		ctx, cancelFunc := context.WithCancel(context.Background())
 		defer cancelFunc()
 		doneChan := make(chan struct{})
-		root, _ := clitest.New(t, "login", client.URL.String(), "--force-tty", "--no-open")
+		root, _ := clitest.New(t, "login", client.URL.String(), "--no-open")
 		pty := ptytest.New(t)
-		root.SetIn(pty.Input().Reader)
+		root.SetIn(pty.Input())
 		root.SetOut(pty.Output())
 		go func() {
 			defer close(doneChan)

diff --git a/cli/root.go b/cli/root.go
@@ -13,6 +13,7 @@ import (
 	"github.com/coder/coder/cli/cliui"
 	"github.com/coder/coder/cli/config"
 	"github.com/coder/coder/codersdk"
+	"github.com/coder/coder/pty"
 )
 
 var (
@@ -141,9 +142,11 @@ func isTTY(cmd *cobra.Command) bool {
 	if forceTty && err == nil {
 		return true
 	}
-
-	reader := cmd.InOrStdin()
-	file, ok := reader.(*os.File)
+	_, ok := cmd.InOrStdin().(pty.ReadWriter)
+	if ok {
+		return true
+	}
+	file, ok := cmd.InOrStdin().(*os.File)
 	if !ok {
 		return false
 	}

diff --git a/go.mod b/go.mod
@@ -17,6 +17,9 @@ replace github.com/chzyer/readline => github.com/kylecarbs/readline v0.0.0-20220
 // opencensus-go leaks a goroutine by default.
 replace go.opencensus.io => github.com/kylecarbs/opencensus-go v0.23.1-0.20220307014935-4d0325a68f8b
 
+// Required until https://github.com/charmbracelet/bubbletea/pull/258 is merged.
+replace github.com/charmbracelet/bubbletea => github.com/kylecarbs/bubbletea v0.20.1-0.20220315224508-b2708bc3e995
+
 // These are to allow embedding the cloudflared quick-tunnel CLI.
 // Required until https://github.com/cloudflare/cloudflared/pull/597 is merged.
 replace github.com/cloudflare/cloudflared => github.com/kylecarbs/cloudflared v0.0.0-20220311054120-ea109c6bf7be
@@ -59,6 +62,7 @@ require (
 	github.com/mattn/go-isatty v0.0.14
 	github.com/mitchellh/mapstructure v1.4.3
 	github.com/moby/moby v20.10.13+incompatible
+	github.com/muesli/cancelreader v0.1.0
 	github.com/ory/dockertest/v3 v3.8.1
 	github.com/pion/datachannel v1.5.2
 	github.com/pion/logging v0.2.2

diff --git a/go.sum b/go.sum
@@ -316,9 +316,6 @@ github.com/cespare/xxhash/v2 v2.1.2 h1:YRXhKfTDauu4ajMg1TPgFO5jnlC2HCbmLXMcTG5cb
 github.com/cespare/xxhash/v2 v2.1.2/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
 github.com/charmbracelet/bubbles v0.10.3 h1:fKarbRaObLn/DCsZO4Y3vKCwRUzynQD9L+gGev1E/ho=
 github.com/charmbracelet/bubbles v0.10.3/go.mod h1:jOA+DUF1rjZm7gZHcNyIVW+YrBPALKfpGVdJu8UiJsA=
-github.com/charmbracelet/bubbletea v0.19.3/go.mod h1:VuXF2pToRxDUHcBUcPmCRUHRvFATM4Ckb/ql1rBl3KA=
-github.com/charmbracelet/bubbletea v0.20.0 h1:/b8LEPgCbNr7WWZ2LuE/BV1/r4t5PyYJtDb+J3vpwxc=
-github.com/charmbracelet/bubbletea v0.20.0/go.mod h1:zpkze1Rioo4rJELjRyGlm9T2YNou1Fm4LIJQSa5QMEM=
 github.com/charmbracelet/charm v0.10.3 h1:UxNC2zNEADn318LtFZ7jQIhwsEccCMi8teQHvf7eVh8=
 github.com/charmbracelet/charm v0.10.3/go.mod h1:cIbe8nSGNfrLJCAGdHsIp2cZT51IUoQ5KPeax9IUnmU=
 github.com/charmbracelet/harmonica v0.1.0/go.mod h1:KSri/1RMQOZLbw7AHqgcBycp8pgJnQMYYT8QZRqZ1Ao=
@@ -1125,6 +1122,8 @@ github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/ktrysmt/go-bitbucket v0.6.4/go.mod h1:9u0v3hsd2rqCHRIpbir1oP7F58uo5dq19sBYvuMoyQ4=
+github.com/kylecarbs/bubbletea v0.20.1-0.20220315224508-b2708bc3e995 h1:hM3bDpv1js78mEApavhwcD21UmF5ZRqA1E86mVx4Shk=
+github.com/kylecarbs/bubbletea v0.20.1-0.20220315224508-b2708bc3e995/go.mod h1:HzsvMPPhvPTYUeAs6ts2/UIybIEzTuikLVvtF+QlQX8=
 github.com/kylecarbs/cloudflared v0.0.0-20220311054120-ea109c6bf7be h1:kl9byH/iaZJ99iJbSAFXjJ8jBpg6TLk6L2/73uSV8wU=
 github.com/kylecarbs/cloudflared v0.0.0-20220311054120-ea109c6bf7be/go.mod h1:4chGYq3uDzeHSpht2LFNZc/8ulHhMW9MvHPvzT5aZx8=
 github.com/kylecarbs/opencensus-go v0.23.1-0.20220307014935-4d0325a68f8b h1:1Y1X6aR78kMEQE1iCjQodB3lA7VO4jB88Wf8ZrzXSsA=
@@ -1289,6 +1288,8 @@ github.com/mrunalp/fileutils v0.5.0/go.mod h1:M1WthSahJixYnrXQl/DFQuteStB1weuxD2
 github.com/muesli/ansi v0.0.0-20211018074035-2e021307bc4b/go.mod h1:fQuZ0gauxyBcmsdE3ZT4NasjaRdxmbCS0jRHsrWu3Ho=
 github.com/muesli/ansi v0.0.0-20211031195517-c9f0611b6c70 h1:kMlmsLSbjkikxQJ1IPwaM+7LJ9ltFu/fi8CRzvSnQmA=
 github.com/muesli/ansi v0.0.0-20211031195517-c9f0611b6c70/go.mod h1:fQuZ0gauxyBcmsdE3ZT4NasjaRdxmbCS0jRHsrWu3Ho=
+github.com/muesli/cancelreader v0.1.0 h1:JJUlDdVrpPeziADbj9WoG8dYfym03lsJNeOUGFwuBAs=
+github.com/muesli/cancelreader v0.1.0/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo=
 github.com/muesli/go-app-paths v0.2.1/go.mod h1:SxS3Umca63pcFcLtbjVb+J0oD7cl4ixQWoBKhGEtEho=
 github.com/muesli/reflow v0.2.1-0.20210115123740-9e1d0d53df68/go.mod h1:Xk+z4oIWdQqJzsxyjgl3P22oYZnHdZ8FFTHAQQt5BMQ=
 github.com/muesli/reflow v0.3.0 h1:IFsN6K9NfGtjeggFP+68I4chLZV2yIKsXJFNZ+eWh6s=
@@ -2138,6 +2139,7 @@ golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220111092808-5a964db01320/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220128215802-99c3d69c2c27/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220204135822-1c1b9b1eba6a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220310020820-b874c991c1a5 h1:y/woIyUBFbpQGKS0u1aHF/40WUDnek3fPOyD08H5Vng=
 golang.org/x/sys v0.0.0-20220310020820-b874c991c1a5/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

diff --git a/pty/pty.go b/pty/pty.go
@@ -4,18 +4,6 @@ import (
 	"io"
 )
 
-const (
-	// See: https://man7.org/linux/man-pages/man3/tcflow.3.html
-	//  (004, EOT, Ctrl-D) End-of-file character (EOF).  More
-	// precisely: this character causes the pending tty buffer to
-	// be sent to the waiting user program without waiting for
-	// end-of-line.  If it is the first character of the line,
-	// the read(2) in the user program returns 0, which signifies
-	// end-of-file.  Recognized when ICANON is set, and then not
-	// passed as input.
-	VEOF byte = 4
-)
-
 // PTY is a minimal interface for interacting with a TTY.
 type PTY interface {
 	io.Closer

diff --git a/pty/pty_other.go b/pty/pty_other.go
@@ -8,6 +8,7 @@ import (
 	"sync"
 
 	"github.com/creack/pty"
+	"github.com/muesli/cancelreader"
 )
 
 func newPty() (PTY, error) {
@@ -65,3 +66,7 @@ func (p *otherPty) Close() error {
 	}
 	return nil
 }
+
+func (rw ReadWriter) CancelReader() (cancelreader.CancelReader, error) {
+	return cancelreader.NewReader(rw.Reader)
+}
diff --git a/pty/pty_windows.go b/pty/pty_windows.go
@@ -10,6 +10,7 @@ import (
 
 	"golang.org/x/sys/windows"
 
+	"github.com/muesli/cancelreader"
 	"golang.org/x/xerrors"
 )
 
@@ -107,3 +108,7 @@ func (p *ptyWindows) Close() error {
 
 	return nil
 }
+
+func (rw ReadWriter) CancelReader() (cancelreader.CancelReader, error) {
+	return nil, xerrors.New("not implemented")
+}
diff --git a/pty/ptytest/ptytest.go b/pty/ptytest/ptytest.go
@@ -13,6 +13,7 @@ import (
 	"unicode/utf8"
 
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
 
 	"github.com/coder/coder/pty"
 )
@@ -21,11 +22,49 @@ var (
 	// Used to ensure terminal output doesn't have anything crazy!
 	// See: https://stackoverflow.com/a/29497680
 	stripAnsi = regexp.MustCompile("[\u001B\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[a-zA-Z\\d]*)*)?\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PRZcf-ntqry=><~]))")
+
+	// See: https://man7.org`/linux/man-pages/man3/tcflow.3.html
+	//  (004, EOT, Ctrl-D) End-of-file character (EOF).  More
+	// precisely: this character causes the pending tty buffer to
+	// be sent to the waiting user program without waiting for
+	// end-of-line.  If it is the first character of the line,
+	// the read(2) in the user program returns 0, which signifies
+	// end-of-file.  Recognized when ICANON is set, and then not
+	// passed as input.
+	VEOF byte = 4
+
+	KeyUp    = []byte{0x1b, '[', 'A'}
+	KeyDown  = []byte{0x1b, '[', 'B'}
+	KeyRight = []byte{0x1b, '[', 'C'}
+	KeyLeft  = []byte{0x1b, '[', 'D'}
 )
 
 func New(t *testing.T) *PTY {
 	ptty, err := pty.New()
 	require.NoError(t, err)
+
+	// In testing we want to avoid raw mode. It takes away our
+	// ability to use VEOF to flush the terminal data.
+	//
+	// If we find another way to flush PTY data to the TTY, we
+	// can avoid using VEOF entirely.
+	if runtime.GOOS != "windows" {
+		// On non-Windows operating systems, the pseudo-terminal
+		// converts a carriage-return into a newline.
+		//
+		// We send unescaped input, so any terminal translations
+		// result in inconsistent behavior.
+		ptyFile, valid := ptty.Input().Reader.(*os.File)
+		require.True(t, valid, "The pty input must be a file!")
+		ptyFileFd := int(ptyFile.Fd())
+
+		state, err := unix.IoctlGetTermios(ptyFileFd, unix.TCGETS)
+		require.NoError(t, err)
+		state.Iflag &^= unix.ICRNL
+		err = unix.IoctlSetTermios(ptyFileFd, unix.TCSETS, state)
+		require.NoError(t, err)
+	}
+
 	return create(t, ptty)
 }
 
@@ -92,11 +131,23 @@ func (p *PTY) ExpectMatch(str string) string {
 	return buffer.String()
 }
 
+func (p *PTY) Write(data []byte) {
+	_, err := p.Input().Write(append(data, VEOF))
+	require.NoError(p.t, err)
+}
+
 func (p *PTY) WriteLine(str string) {
-	newline := []byte{pty.VEOF, '\r'}
+	_, err := p.Input().Write(append([]byte(str), VEOF))
+	require.NoError(p.t, err)
+	p.WriteEnter()
+}
+
+func (p *PTY) WriteEnter() {
+	newline := []byte{'\r'}
 	if runtime.GOOS == "windows" {
 		newline = append(newline, '\n')
 	}
-	_, err := p.Input().Write(append([]byte(str), []byte(newline)...))
+	newline = append(newline, VEOF)
+	_, err := p.Input().Write(newline)
 	require.NoError(p.t, err)
 }