Skip to content

Commit 72ec5be

Browse files
samcoesamcoe
authored
Merge pull request cli#2016 from cli/filter-git-push-output
Filter out unwanted stderr output during git push commands
2 parents e3fbe47 + 9acbf8a commit 72ec5be

File tree

4 files changed

+256
-16
lines changed

4 files changed

+256
-16
lines changed

‎git/git.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"errors"
66
"fmt"
7+
"io"
78
"net/url"
89
"os"
910
"os/exec"
@@ -162,10 +163,10 @@ func CommitBody(sha string) (string, error) {
162163
}
163164

164165
// Push publishes a git ref to a remote and sets up upstream configuration
165-
func Push(remote string, ref string) error {
166+
func Push(remote string, ref string, cmdOut, cmdErr io.Writer) error {
166167
pushCmd := GitCommand("push", "--set-upstream", remote, ref)
167-
pushCmd.Stdout = os.Stdout
168-
pushCmd.Stderr = os.Stderr
168+
pushCmd.Stdout = cmdOut
169+
pushCmd.Stderr = cmdErr
169170
return run.PrepareCmd(pushCmd).Run()
170171
}
171172

‎pkg/cmd/pr/create/create.go‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"net/http"
77
"net/url"
8+
"regexp"
89
"strings"
910
"time"
1011

@@ -426,21 +427,33 @@ func createRun(opts *CreateOptions) error {
426427

427428
// automatically push the branch if it hasn't been pushed anywhere yet
428429
if isPushEnabled {
429-
pushTries := 0
430-
maxPushTries := 3
431-
for {
432-
if err := git.Push(headRemote.Name, fmt.Sprintf("HEAD:%s", headBranch)); err != nil {
433-
if didForkRepo && pushTries < maxPushTries {
434-
pushTries++
435-
// first wait 2 seconds after forking, then 4s, then 6s
436-
waitSeconds := 2 * pushTries
437-
fmt.Fprintf(opts.IO.ErrOut, "waiting %s before retrying...\n", utils.Pluralize(waitSeconds, "second"))
438-
time.Sleep(time.Duration(waitSeconds) * time.Second)
439-
continue
430+
pushBranch := func() error {
431+
pushTries := 0
432+
maxPushTries := 3
433+
for {
434+
r := NewRegexpWriter(opts.IO.ErrOut, gitPushRegexp, "")
435+
defer r.Flush()
436+
cmdErr := r
437+
cmdOut := opts.IO.Out
438+
if err := git.Push(headRemote.Name, fmt.Sprintf("HEAD:%s", headBranch), cmdOut, cmdErr); err != nil {
439+
if didForkRepo && pushTries < maxPushTries {
440+
pushTries++
441+
// first wait 2 seconds after forking, then 4s, then 6s
442+
waitSeconds := 2 * pushTries
443+
fmt.Fprintf(opts.IO.ErrOut, "waiting %s before retrying...\n", utils.Pluralize(waitSeconds, "second"))
444+
time.Sleep(time.Duration(waitSeconds) * time.Second)
445+
continue
446+
}
447+
return err
440448
}
441-
return err
449+
break
442450
}
443-
break
451+
return nil
452+
}
453+
454+
err := pushBranch()
455+
if err != nil {
456+
return err
444457
}
445458
}
446459

@@ -561,3 +574,5 @@ func generateCompareURL(r ghrepo.Interface, base, head, title, body string, assi
561574
}
562575
return url, nil
563576
}
577+
578+
var gitPushRegexp = regexp.MustCompile("^remote: (Create a pull request.*by visiting|[[:space:]]*https://.*/pull/new/).*\n?$")

‎pkg/cmd/pr/create/regexp_writer.go‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package create
2+
3+
import (
4+
"bytes"
5+
"io"
6+
"regexp"
7+
)
8+
9+
func NewRegexpWriter(out io.Writer, re *regexp.Regexp, repl string) *RegexpWriter {
10+
return &RegexpWriter{out: out, re: *re, repl: repl}
11+
}
12+
13+
type RegexpWriter struct {
14+
out io.Writer
15+
re regexp.Regexp
16+
repl string
17+
buf []byte
18+
}
19+
20+
func (s *RegexpWriter) Write(data []byte) (int, error) {
21+
if len(data) == 0 {
22+
return 0, nil
23+
}
24+
25+
filtered := []byte{}
26+
repl := []byte(s.repl)
27+
lines := bytes.SplitAfter(data, []byte("\n"))
28+
29+
if len(s.buf) > 0 {
30+
lines[0] = append(s.buf, lines[0]...)
31+
}
32+
33+
for i, line := range lines {
34+
if i == len(lines) {
35+
s.buf = line
36+
} else {
37+
f := s.re.ReplaceAll(line, repl)
38+
if len(f) > 0 {
39+
filtered = append(filtered, f...)
40+
}
41+
}
42+
}
43+
44+
if len(filtered) != 0 {
45+
_, err := s.out.Write(filtered)
46+
if err != nil {
47+
return 0, err
48+
}
49+
}
50+
51+
return len(data), nil
52+
}
53+
54+
func (s *RegexpWriter) Flush() (int, error) {
55+
if len(s.buf) > 0 {
56+
repl := []byte(s.repl)
57+
filtered := s.re.ReplaceAll(s.buf, repl)
58+
if len(filtered) > 0 {
59+
return s.out.Write(filtered)
60+
}
61+
}
62+
63+
return 0, nil
64+
}

‎pkg/cmd/pr/create/regexp_writer_test.go‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
package create
2+
3+
import (
4+
"bytes"
5+
"regexp"
6+
"testing"
7+
8+
"github.com/MakeNowJust/heredoc"
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
func Test_Write(t *testing.T) {
13+
type input struct {
14+
in []string
15+
re *regexp.Regexp
16+
repl string
17+
}
18+
type output struct {
19+
wantsErr bool
20+
out string
21+
length int
22+
}
23+
tests := []struct {
24+
name string
25+
input input
26+
output output
27+
}{
28+
{
29+
name: "single line input",
30+
input: input{
31+
in: []string{"some input line that has wrong information"},
32+
re: regexp.MustCompile("wrong"),
33+
repl: "right",
34+
},
35+
output: output{
36+
wantsErr: false,
37+
out: "some input line that has right information",
38+
length: 42,
39+
},
40+
},
41+
{
42+
name: "multiple line input",
43+
input: input{
44+
in: []string{"multiple lines\nin this\ninput lines"},
45+
re: regexp.MustCompile("lines"),
46+
repl: "tests",
47+
},
48+
output: output{
49+
wantsErr: false,
50+
out: "multiple tests\nin this\ninput tests",
51+
length: 34,
52+
},
53+
},
54+
{
55+
name: "no matches",
56+
input: input{
57+
in: []string{"this line has no matches"},
58+
re: regexp.MustCompile("wrong"),
59+
repl: "right",
60+
},
61+
output: output{
62+
wantsErr: false,
63+
out: "this line has no matches",
64+
length: 24,
65+
},
66+
},
67+
{
68+
name: "no output",
69+
input: input{
70+
in: []string{"remove this whole line"},
71+
re: regexp.MustCompile("^remove.*$"),
72+
repl: "",
73+
},
74+
output: output{
75+
wantsErr: false,
76+
out: "",
77+
length: 22,
78+
},
79+
},
80+
{
81+
name: "no input",
82+
input: input{
83+
in: []string{""},
84+
re: regexp.MustCompile("remove"),
85+
repl: "",
86+
},
87+
output: output{
88+
wantsErr: false,
89+
out: "",
90+
length: 0,
91+
},
92+
},
93+
{
94+
name: "multiple lines removed",
95+
input: input{
96+
in: []string{"begining line\nremove this whole line\nremove this one also\nnot this one"},
97+
re: regexp.MustCompile("(?s)^remove.*$"),
98+
repl: "",
99+
},
100+
output: output{
101+
wantsErr: false,
102+
out: "begining line\nnot this one",
103+
length: 70,
104+
},
105+
},
106+
{
107+
name: "removes remote from git push output",
108+
input: input{
109+
in: []string{heredoc.Doc(`
110+
output: some information
111+
remote:
112+
remote: Create a pull request for 'regex' on GitHub by visiting:
113+
remote: https://github.com/owner/repo/pull/new/regex
114+
remote:
115+
output: more information
116+
`)},
117+
re: regexp.MustCompile("^remote: (Create a pull request.*by visiting|[[:space:]]*https://.*/pull/new/).*\n?$"),
118+
repl: "",
119+
},
120+
output: output{
121+
wantsErr: false,
122+
out: "output: some information\nremote:\nremote:\noutput: more information\n",
123+
length: 189,
124+
},
125+
},
126+
{
127+
name: "multiple writes",
128+
input: input{
129+
in: []string{"first write\n", "second write ", "third write"},
130+
re: regexp.MustCompile("write"),
131+
repl: "read",
132+
},
133+
output: output{
134+
wantsErr: false,
135+
out: "first read\nsecond read third read",
136+
length: 36,
137+
},
138+
},
139+
}
140+
141+
for _, tt := range tests {
142+
out := &bytes.Buffer{}
143+
writer := NewRegexpWriter(out, tt.input.re, tt.input.repl)
144+
t.Run(tt.name, func(t *testing.T) {
145+
length := 0
146+
for _, in := range tt.input.in {
147+
l, err := writer.Write([]byte(in))
148+
length = length + l
149+
if tt.output.wantsErr {
150+
assert.Error(t, err)
151+
return
152+
}
153+
assert.NoError(t, err)
154+
}
155+
writer.Flush()
156+
assert.Equal(t, tt.output.out, out.String())
157+
assert.Equal(t, tt.output.length, length)
158+
})
159+
}
160+
}

0 commit comments

Comments
 (0)