Skip to content

Commit e2e8d69

Browse files
authored
Fix closing IOStreams.Out after finishing writing to the pager (cli#6210)
1 parent a663699 commit e2e8d69

2 files changed

Lines changed: 53 additions & 3 deletions

File tree

‎pkg/iostreams/iostreams.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,9 @@ func (s *IOStreams) StartPager() error {
214214
if err != nil {
215215
return err
216216
}
217-
s.Out = &fdWriter{
218-
fd: s.Out.Fd(),
219-
Writer: &pagerWriter{pagedOut},
217+
s.Out = &fdWriteCloser{
218+
fd: s.Out.Fd(),
219+
WriteCloser: &pagerWriter{pagedOut},
220220
}
221221
err = pagerCmd.Start()
222222
if err != nil {
@@ -231,6 +231,7 @@ func (s *IOStreams) StopPager() {
231231
return
232232
}
233233

234+
// if a pager was started, we're guaranteed to have a WriteCloser
234235
_ = s.Out.(io.WriteCloser).Close()
235236
_, _ = s.pagerProcess.Wait()
236237
s.pagerProcess = nil
@@ -521,6 +522,16 @@ func (w *fdWriter) Fd() uintptr {
521522
return w.fd
522523
}
523524

525+
// fdWriteCloser represents a wrapped stdout Writer that preserves the original file descriptor
526+
type fdWriteCloser struct {
527+
io.WriteCloser
528+
fd uintptr
529+
}
530+
531+
func (w *fdWriteCloser) Fd() uintptr {
532+
return w.fd
533+
}
534+
524535
// fdWriter represents a wrapped stdin ReadCloser that preserves the original file descriptor
525536
type fdReader struct {
526537
io.ReadCloser

‎pkg/iostreams/iostreams_test.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package iostreams
22

33
import (
4+
"bufio"
45
"errors"
56
"fmt"
67
"io"
8+
"os"
79
"testing"
810
)
911

@@ -89,3 +91,40 @@ func TestStopAlternateScreenBuffer(t *testing.T) {
8991
t.Errorf("after IOStreams.StopAlternateScreenBuffer() got %q, want %q", got, want)
9092
}
9193
}
94+
95+
func TestIOStreams_pager(t *testing.T) {
96+
t.Skip("TODO: fix this test in race detection mode")
97+
ios, _, stdout, _ := Test()
98+
ios.SetStdoutTTY(true)
99+
ios.SetPager(fmt.Sprintf("%s -test.run=TestHelperProcess --", os.Args[0]))
100+
t.Setenv("GH_WANT_HELPER_PROCESS", "1")
101+
if err := ios.StartPager(); err != nil {
102+
t.Fatal(err)
103+
}
104+
if _, err := fmt.Fprintln(ios.Out, "line1"); err != nil {
105+
t.Errorf("error writing line 1: %v", err)
106+
}
107+
if _, err := fmt.Fprintln(ios.Out, "line2"); err != nil {
108+
t.Errorf("error writing line 2: %v", err)
109+
}
110+
ios.StopPager()
111+
wants := "pager: line1\npager: line2\n"
112+
if got := stdout.String(); got != wants {
113+
t.Errorf("expected %q, got %q", wants, got)
114+
}
115+
}
116+
117+
func TestHelperProcess(t *testing.T) {
118+
if os.Getenv("GH_WANT_HELPER_PROCESS") != "1" {
119+
return
120+
}
121+
scanner := bufio.NewScanner(os.Stdin)
122+
for scanner.Scan() {
123+
fmt.Printf("pager: %s\n", scanner.Text())
124+
}
125+
if err := scanner.Err(); err != nil {
126+
fmt.Fprintf(os.Stderr, "error reading stdin: %v", err)
127+
os.Exit(1)
128+
}
129+
os.Exit(0)
130+
}

0 commit comments

Comments
 (0)