X Tutup
Skip to content

Commit b6fa883

Browse files
mislavmislav
committed
Ensure that commands print to a colorable output
If a command does `fmt.Print(...)` for output that contains ANSI color codes, this not safe on Windows. We have to ensure that we always use the `fmt.Fprint*` family of functions with a writer that was transformed using `utils.NewColorable()`.
1 parent 4148cf7 commit b6fa883

File tree

5 files changed

+69
-43
lines changed

5 files changed

+69
-43
lines changed

‎command/issue.go‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package command
22

33
import (
44
"fmt"
5+
"io"
56
"os"
67
"strconv"
78
"strings"
@@ -94,12 +95,15 @@ func issueList(cmd *cobra.Command, args []string) error {
9495
return err
9596
}
9697

98+
out := cmd.OutOrStdout()
99+
colorOut := colorableOut(cmd)
100+
97101
if len(issues) == 0 {
98-
printMessage("There are no open issues")
102+
printMessage(colorOut, "There are no open issues")
99103
return nil
100104
}
101105

102-
table := utils.NewTablePrinter(cmd.OutOrStdout())
106+
table := utils.NewTablePrinter(out)
103107
for _, issue := range issues {
104108
issueNum := strconv.Itoa(issue.Number)
105109
if table.IsTTY() {
@@ -141,30 +145,32 @@ func issueStatus(cmd *cobra.Command, args []string) error {
141145
return err
142146
}
143147

144-
printHeader("Issues assigned to you")
148+
out := colorableOut(cmd)
149+
150+
printHeader(out, "Issues assigned to you")
145151
if issuePayload.Assigned != nil {
146-
printIssues(" ", issuePayload.Assigned...)
152+
printIssues(out, " ", issuePayload.Assigned...)
147153
} else {
148154
message := fmt.Sprintf(" There are no issues assgined to you")
149-
printMessage(message)
155+
printMessage(out, message)
150156
}
151-
fmt.Println()
157+
fmt.Fprintln(out)
152158

153-
printHeader("Issues mentioning you")
159+
printHeader(out, "Issues mentioning you")
154160
if len(issuePayload.Mentioned) > 0 {
155-
printIssues(" ", issuePayload.Mentioned...)
161+
printIssues(out, " ", issuePayload.Mentioned...)
156162
} else {
157-
printMessage(" There are no issues mentioning you")
163+
printMessage(out, " There are no issues mentioning you")
158164
}
159-
fmt.Println()
165+
fmt.Fprintln(out)
160166

161-
printHeader("Issues opened by you")
167+
printHeader(out, "Issues opened by you")
162168
if len(issuePayload.Authored) > 0 {
163-
printIssues(" ", issuePayload.Authored...)
169+
printIssues(out, " ", issuePayload.Authored...)
164170
} else {
165-
printMessage(" There are no issues opened by you")
171+
printMessage(out, " There are no issues opened by you")
166172
}
167-
fmt.Println()
173+
fmt.Fprintln(out)
168174

169175
return nil
170176
}
@@ -255,14 +261,14 @@ func issueCreate(cmd *cobra.Command, args []string) error {
255261
return nil
256262
}
257263

258-
func printIssues(prefix string, issues ...api.Issue) {
264+
func printIssues(w io.Writer, prefix string, issues ...api.Issue) {
259265
for _, issue := range issues {
260266
number := utils.Green("#" + strconv.Itoa(issue.Number))
261267
coloredLabels := labelList(issue)
262268
if coloredLabels != "" {
263269
coloredLabels = utils.Gray(fmt.Sprintf(" (%s)", coloredLabels))
264270
}
265-
fmt.Printf("%s%s %s%s\n", prefix, number, truncate(70, issue.Title), coloredLabels)
271+
fmt.Fprintf(w, "%s%s %s%s\n", prefix, number, truncate(70, issue.Title), coloredLabels)
266272
}
267273
}
268274

‎command/pr.go‎

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package command
22

33
import (
44
"fmt"
5+
"io"
56
"os"
67
"os/exec"
78
"strconv"
@@ -78,30 +79,32 @@ func prStatus(cmd *cobra.Command, args []string) error {
7879
return err
7980
}
8081

81-
printHeader("Current branch")
82+
out := colorableOut(cmd)
83+
84+
printHeader(out, "Current branch")
8285
if prPayload.CurrentPR != nil {
83-
printPrs(*prPayload.CurrentPR)
86+
printPrs(out, *prPayload.CurrentPR)
8487
} else {
8588
message := fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentBranch+"]"))
86-
printMessage(message)
89+
printMessage(out, message)
8790
}
88-
fmt.Println()
91+
fmt.Fprintln(out)
8992

90-
printHeader("Created by you")
93+
printHeader(out, "Created by you")
9194
if len(prPayload.ViewerCreated) > 0 {
92-
printPrs(prPayload.ViewerCreated...)
95+
printPrs(out, prPayload.ViewerCreated...)
9396
} else {
94-
printMessage(" You have no open pull requests")
97+
printMessage(out, " You have no open pull requests")
9598
}
96-
fmt.Println()
99+
fmt.Fprintln(out)
97100

98-
printHeader("Requesting a code review from you")
101+
printHeader(out, "Requesting a code review from you")
99102
if len(prPayload.ReviewRequested) > 0 {
100-
printPrs(prPayload.ReviewRequested...)
103+
printPrs(out, prPayload.ReviewRequested...)
101104
} else {
102-
printMessage(" You have no pull requests to review")
105+
printMessage(out, " You have no pull requests to review")
103106
}
104-
fmt.Println()
107+
fmt.Fprintln(out)
105108

106109
return nil
107110
}
@@ -330,15 +333,15 @@ func prCheckout(cmd *cobra.Command, args []string) error {
330333
return nil
331334
}
332335

333-
func printPrs(prs ...api.PullRequest) {
336+
func printPrs(w io.Writer, prs ...api.PullRequest) {
334337
for _, pr := range prs {
335338
prNumber := fmt.Sprintf("#%d", pr.Number)
336-
fmt.Printf(" %s %s %s", utils.Yellow(prNumber), truncate(50, pr.Title), utils.Cyan("["+pr.HeadLabel()+"]"))
339+
fmt.Fprintf(w, " %s %s %s", utils.Yellow(prNumber), truncate(50, pr.Title), utils.Cyan("["+pr.HeadLabel()+"]"))
337340

338341
checks := pr.ChecksStatus()
339342
reviews := pr.ReviewStatus()
340343
if checks.Total > 0 || reviews.ChangesRequested || reviews.Approved {
341-
fmt.Printf("\n ")
344+
fmt.Fprintf(w, "\n ")
342345
}
343346

344347
if checks.Total > 0 {
@@ -354,27 +357,27 @@ func printPrs(prs ...api.PullRequest) {
354357
} else if checks.Passing == checks.Total {
355358
summary = utils.Green("Checks passing")
356359
}
357-
fmt.Printf(" - %s", summary)
360+
fmt.Fprintf(w, " - %s", summary)
358361
}
359362

360363
if reviews.ChangesRequested {
361-
fmt.Printf(" - %s", utils.Red("changes requested"))
364+
fmt.Fprintf(w, " - %s", utils.Red("changes requested"))
362365
} else if reviews.ReviewRequired {
363-
fmt.Printf(" - %s", utils.Yellow("review required"))
366+
fmt.Fprintf(w, " - %s", utils.Yellow("review required"))
364367
} else if reviews.Approved {
365-
fmt.Printf(" - %s", utils.Green("approved"))
368+
fmt.Fprintf(w, " - %s", utils.Green("approved"))
366369
}
367370

368-
fmt.Printf("\n")
371+
fmt.Fprint(w, "\n")
369372
}
370373
}
371374

372-
func printHeader(s string) {
373-
fmt.Println(utils.Bold(s))
375+
func printHeader(w io.Writer, s string) {
376+
fmt.Fprintln(w, utils.Bold(s))
374377
}
375378

376-
func printMessage(s string) {
377-
fmt.Println(utils.Gray(s))
379+
func printMessage(w io.Writer, s string) {
380+
fmt.Fprintln(w, utils.Gray(s))
378381
}
379382

380383
func truncate(maxLength int, title string) string {

‎command/root.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ package command
22

33
import (
44
"fmt"
5+
"io"
56
"os"
67
"strings"
78

89
"github.com/github/gh-cli/api"
910
"github.com/github/gh-cli/context"
11+
"github.com/github/gh-cli/utils"
1012

1113
"github.com/spf13/cobra"
1214
)
@@ -93,3 +95,11 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) {
9395
}
9496
return api.NewClient(opts...), nil
9597
}
98+
99+
func colorableOut(cmd *cobra.Command) io.Writer {
100+
out := cmd.OutOrStdout()
101+
if outFile, isFile := out.(*os.File); isFile {
102+
return utils.NewColorable(outFile)
103+
}
104+
return out
105+
}

‎utils/color.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,19 @@
11
package utils
22

33
import (
4+
"io"
5+
"os"
6+
7+
"github.com/mattn/go-colorable"
48
"github.com/mattn/go-isatty"
59
"github.com/mgutz/ansi"
6-
"os"
710
)
811

12+
// NewColorable returns an output stream that handles ANSI color sequences on Windows
13+
func NewColorable(f *os.File) io.Writer {
14+
return colorable.NewColorable(f)
15+
}
16+
917
func makeColorFunc(color string) func(string) string {
1018
return func(arg string) string {
1119
output := arg

‎utils/table_printer.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"strconv"
99
"strings"
1010

11-
"github.com/mattn/go-colorable"
1211
"github.com/mattn/go-isatty"
1312
"golang.org/x/crypto/ssh/terminal"
1413
)
@@ -37,7 +36,7 @@ func NewTablePrinter(w io.Writer) TablePrinter {
3736
}
3837
}
3938
return &ttyTablePrinter{
40-
out: colorable.NewColorable(outFile),
39+
out: NewColorable(outFile),
4140
maxWidth: ttyWidth,
4241
}
4342
}

0 commit comments

Comments
 (0)
X Tutup