X Tutup
Skip to content

Commit 36ade42

Browse files
vilmibmvilmibm
committed
scriptability improvements: issue commands
This commit is part of work to make gh more scriptable. It includes both some general purpose helpers towards this goal as well as improvements to the issue commands. Other commands will follow. - Adds `utils/terminal.go` for finding out about gh's execution environment - introduces `stubTerminal` for either faking being attached to a tty or not during tests - updates issue commands to behave better when not attached to a tty: - issue list doesn't print fuzzy dates - issue list doesn't print header - issue list prints state explicitly - issue create no longer hangs - issue create fails with clear error unless both -t and -b are specified - issue view prints raw issue body - issue view prints metadata in a consistent, linewise format
1 parent 53ff384 commit 36ade42

File tree

7 files changed

+273
-57
lines changed

7 files changed

+273
-57
lines changed

‎command/issue.go‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,9 @@ func issueList(cmd *cobra.Command, args []string) error {
184184
})
185185

186186
title := listHeader(ghrepo.FullName(baseRepo), "issue", len(listResult.Issues), listResult.TotalCount, hasFilters)
187-
// TODO: avoid printing header if piped to a script
188-
fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title)
187+
if connectedToTerminal(cmd) {
188+
fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title)
189+
}
189190

190191
out := cmd.OutOrStdout()
191192

@@ -278,8 +279,11 @@ func issueView(cmd *cobra.Command, args []string) error {
278279
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
279280
return utils.OpenInBrowser(openURL)
280281
}
281-
out := colorableOut(cmd)
282-
return printIssuePreview(out, issue)
282+
if connectedToTerminal(cmd) {
283+
return printHumanIssuePreview(colorableOut(cmd), issue)
284+
}
285+
286+
return printRawIssuePreview(cmd.OutOrStdout(), issue)
283287
}
284288

285289
func issueStateTitleWithColor(state string) string {
@@ -306,7 +310,28 @@ func listHeader(repoName string, itemName string, matchCount int, totalMatchCoun
306310
return fmt.Sprintf("Showing %d of %s in %s", matchCount, utils.Pluralize(totalMatchCount, itemName), repoName)
307311
}
308312

309-
func printIssuePreview(out io.Writer, issue *api.Issue) error {
313+
func printRawIssuePreview(out io.Writer, issue *api.Issue) error {
314+
assignees := issueAssigneeList(*issue)
315+
labels := issueLabelList(*issue)
316+
projects := issueProjectList(*issue)
317+
318+
// Print empty strings for empty values so the number of metadata lines is always consistent (ie
319+
// so head -n8 can be relied on)
320+
fmt.Fprintf(out, "title:\t%s\n", issue.Title)
321+
fmt.Fprintf(out, "state:\t%s\n", issue.State)
322+
fmt.Fprintf(out, "author:\t%s\n", issue.Author.Login)
323+
fmt.Fprintf(out, "labels:\t%s\n", labels)
324+
fmt.Fprintf(out, "comments:\t%d\n", issue.Comments.TotalCount)
325+
fmt.Fprintf(out, "assignees:\t%s\n", assignees)
326+
fmt.Fprintf(out, "projects:\t%s\n", projects)
327+
fmt.Fprintf(out, "milestone:\t%s\n", issue.Milestone.Title)
328+
329+
fmt.Fprintln(out, "--")
330+
fmt.Fprintln(out, issue.Body)
331+
return nil
332+
}
333+
334+
func printHumanIssuePreview(out io.Writer, issue *api.Issue) error {
310335
now := time.Now()
311336
ago := now.Sub(issue.CreatedAt)
312337

@@ -464,6 +489,10 @@ func issueCreate(cmd *cobra.Command, args []string) error {
464489

465490
interactive := !(cmd.Flags().Changed("title") && cmd.Flags().Changed("body"))
466491

492+
if interactive && !connectedToTerminal(cmd) {
493+
return fmt.Errorf("can't run non-interactively without both --title and --body")
494+
}
495+
467496
if interactive {
468497
var legacyTemplateFile *string
469498
if baseOverride == "" {
@@ -625,9 +654,16 @@ func printIssues(w io.Writer, prefix string, totalCount int, issues []api.Issue)
625654
now := time.Now()
626655
ago := now.Sub(issue.UpdatedAt)
627656
table.AddField(issueNum, nil, colorFuncForState(issue.State))
657+
if !table.IsTTY() {
658+
table.AddField(issue.State, nil, nil)
659+
}
628660
table.AddField(replaceExcessiveWhitespace(issue.Title), nil, nil)
629661
table.AddField(labels, nil, utils.Gray)
630-
table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray)
662+
if table.IsTTY() {
663+
table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray)
664+
} else {
665+
table.AddField(fmt.Sprintf("%d", int(ago.Minutes())), nil, nil)
666+
}
631667
table.EndRow()
632668
}
633669
_ = table.Render()

‎command/issue_test.go‎

Lines changed: 139 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
func TestIssueStatus(t *testing.T) {
2020
initBlankContext("", "OWNER/REPO", "master")
21+
defer stubTerminal(true)()
2122
http := initFakeHTTP()
2223
http.StubRepoResponse("OWNER", "REPO")
2324
http.Register(
@@ -107,8 +108,31 @@ func TestIssueStatus_disabledIssues(t *testing.T) {
107108
}
108109
}
109110

110-
func TestIssueList(t *testing.T) {
111+
func TestIssueList_nontty(t *testing.T) {
111112
initBlankContext("", "OWNER/REPO", "master")
113+
defer stubTerminal(false)()
114+
http := initFakeHTTP()
115+
http.StubRepoResponse("OWNER", "REPO")
116+
117+
jsonFile, _ := os.Open("../test/fixtures/issueList.json")
118+
defer jsonFile.Close()
119+
http.StubResponse(200, jsonFile)
120+
121+
output, err := RunCommand("issue list")
122+
if err != nil {
123+
t.Errorf("error running command `issue list`: %v", err)
124+
}
125+
126+
eq(t, output.Stderr(), "")
127+
test.ExpectLines(t, output.String(),
128+
`1[\t]+number won[\t]+label[\t]+\d+`,
129+
`2[\t]+number too[\t]+label[\t]+\d+`,
130+
`4[\t]+number fore[\t]+label[\t]+\d+`)
131+
}
132+
133+
func TestIssueList_tty(t *testing.T) {
134+
initBlankContext("", "OWNER/REPO", "master")
135+
defer stubTerminal(true)()
112136
http := initFakeHTTP()
113137
http.StubRepoResponse("OWNER", "REPO")
114138
http.Register(
@@ -125,21 +149,14 @@ Showing 3 of 3 issues in OWNER/REPO
125149
126150
`)
127151

128-
expectedIssues := []*regexp.Regexp{
129-
regexp.MustCompile(`(?m)^1\t.*won`),
130-
regexp.MustCompile(`(?m)^2\t.*too`),
131-
regexp.MustCompile(`(?m)^4\t.*fore`),
132-
}
133-
134-
for _, r := range expectedIssues {
135-
if !r.MatchString(output.String()) {
136-
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
137-
return
138-
}
139-
}
152+
test.ExpectLines(t, output.String(),
153+
"number won",
154+
"number too",
155+
"number fore")
140156
}
141157

142-
func TestIssueList_withFlags(t *testing.T) {
158+
func TestIssueList_tty_withFlags(t *testing.T) {
159+
defer stubTerminal(true)()
143160
initBlankContext("", "OWNER/REPO", "master")
144161
http := initFakeHTTP()
145162
http.StubRepoResponse("OWNER", "REPO")
@@ -296,7 +313,92 @@ func TestIssueView_web_numberArgWithHash(t *testing.T) {
296313
eq(t, url, "https://github.com/OWNER/REPO/issues/123")
297314
}
298315

299-
func TestIssueView_Preview(t *testing.T) {
316+
func TestIssueView_nontty_Preview(t *testing.T) {
317+
defer stubTerminal(false)()
318+
tests := map[string]struct {
319+
ownerRepo string
320+
command string
321+
fixture string
322+
expectedOutputs []string
323+
}{
324+
"Open issue without metadata": {
325+
ownerRepo: "master",
326+
command: "issue view 123",
327+
fixture: "../test/fixtures/issueView_preview.json",
328+
expectedOutputs: []string{
329+
`title:\tix of coins`,
330+
`state:\tOPEN`,
331+
`comments:\t9`,
332+
`author:\tmarseilles`,
333+
`assignees:`,
334+
`\*\*bold story\*\*`,
335+
},
336+
},
337+
"Open issue with metadata": {
338+
ownerRepo: "master",
339+
command: "issue view 123",
340+
fixture: "../test/fixtures/issueView_previewWithMetadata.json",
341+
expectedOutputs: []string{
342+
`title:\tix of coins`,
343+
`assignees:\tmarseilles, monaco`,
344+
`author:\tmarseilles`,
345+
`state:\tOPEN`,
346+
`comments:\t9`,
347+
`labels:\tone, two, three, four, five`,
348+
`projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
349+
`milestone:\tuluru\n`,
350+
`\*\*bold story\*\*`,
351+
},
352+
},
353+
"Open issue with empty body": {
354+
ownerRepo: "master",
355+
command: "issue view 123",
356+
fixture: "../test/fixtures/issueView_previewWithEmptyBody.json",
357+
expectedOutputs: []string{
358+
`title:\tix of coins`,
359+
`state:\tOPEN`,
360+
`author:\tmarseilles`,
361+
`labels:\ttarot`,
362+
},
363+
},
364+
"Closed issue": {
365+
ownerRepo: "master",
366+
command: "issue view 123",
367+
fixture: "../test/fixtures/issueView_previewClosedState.json",
368+
expectedOutputs: []string{
369+
`title:\tix of coins`,
370+
`state:\tCLOSED`,
371+
`\*\*bold story\*\*`,
372+
`author:\tmarseilles`,
373+
`labels:\ttarot`,
374+
`\*\*bold story\*\*`,
375+
},
376+
},
377+
}
378+
for name, tc := range tests {
379+
t.Run(name, func(t *testing.T) {
380+
initBlankContext("", "OWNER/REPO", tc.ownerRepo)
381+
http := initFakeHTTP()
382+
http.StubRepoResponse("OWNER", "REPO")
383+
384+
jsonFile, _ := os.Open(tc.fixture)
385+
defer jsonFile.Close()
386+
http.StubResponse(200, jsonFile)
387+
388+
output, err := RunCommand(tc.command)
389+
if err != nil {
390+
t.Errorf("error running command `%v`: %v", tc.command, err)
391+
}
392+
393+
eq(t, output.Stderr(), "")
394+
395+
test.ExpectLines(t, output.String(), tc.expectedOutputs...)
396+
})
397+
}
398+
}
399+
400+
func TestIssueView_tty_Preview(t *testing.T) {
401+
defer stubTerminal(true)()
300402
tests := map[string]struct {
301403
ownerRepo string
302404
command string
@@ -448,6 +550,28 @@ func TestIssueView_web_urlArg(t *testing.T) {
448550
eq(t, url, "https://github.com/OWNER/REPO/issues/123")
449551
}
450552

553+
func TestIssueCreate_nontty_error(t *testing.T) {
554+
defer stubTerminal(false)()
555+
initBlankContext("", "OWNER/REPO", "master")
556+
http := initFakeHTTP()
557+
http.StubRepoResponse("OWNER", "REPO")
558+
559+
http.StubResponse(200, bytes.NewBufferString(`
560+
{ "data": { "repository": {
561+
"id": "REPOID",
562+
"hasIssuesEnabled": true
563+
} } }
564+
`))
565+
566+
_, err := RunCommand(`issue create -t hello`)
567+
if err == nil {
568+
t.Fatal("expected error running command `issue create`")
569+
}
570+
571+
assert.Equal(t, "can't run non-interactively without both --title and --body", err.Error())
572+
573+
}
574+
451575
func TestIssueCreate(t *testing.T) {
452576
initBlankContext("", "OWNER/REPO", "master")
453577
http := initFakeHTTP()

‎command/root.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,3 +407,7 @@ func ExpandAlias(args []string) ([]string, error) {
407407

408408
return args[1:], nil
409409
}
410+
411+
func connectedToTerminal(cmd *cobra.Command) bool {
412+
return utils.IsTerminal(cmd.InOrStdin()) && utils.IsTerminal(cmd.OutOrStdout())
413+
}

‎command/testing.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/cli/cli/context"
1313
"github.com/cli/cli/internal/config"
1414
"github.com/cli/cli/pkg/httpmock"
15+
"github.com/cli/cli/utils"
1516
"github.com/google/shlex"
1617
"github.com/spf13/pflag"
1718
)
@@ -169,3 +170,26 @@ func (s errorStub) Output() ([]byte, error) {
169170
func (s errorStub) Run() error {
170171
return errors.New(s.message)
171172
}
173+
174+
func stubTerminal(connected bool) func() {
175+
isTerminal := utils.IsTerminal
176+
utils.IsTerminal = func(_ interface{}) bool {
177+
return connected
178+
}
179+
180+
terminalSize := utils.TerminalSize
181+
if connected {
182+
utils.TerminalSize = func(_ interface{}) (int, int, error) {
183+
return 80, 20, nil
184+
}
185+
} else {
186+
utils.TerminalSize = func(_ interface{}) (int, int, error) {
187+
return 0, 0, fmt.Errorf("terminal connection stubbed to false")
188+
}
189+
}
190+
191+
return func() {
192+
utils.IsTerminal = isTerminal
193+
utils.TerminalSize = terminalSize
194+
}
195+
}

‎utils/color.go‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"os"
66

77
"github.com/mattn/go-colorable"
8-
"github.com/mattn/go-isatty"
98
"github.com/mgutz/ansi"
109
)
1110

@@ -23,22 +22,12 @@ var (
2322
Bold = makeColorFunc("default+b")
2423
)
2524

26-
func isStdoutTerminal() bool {
27-
if !checkedTerminal {
28-
_isStdoutTerminal = IsTerminal(os.Stdout)
29-
checkedTerminal = true
30-
}
31-
return _isStdoutTerminal
32-
}
33-
34-
// IsTerminal reports whether the file descriptor is connected to a terminal
35-
func IsTerminal(f *os.File) bool {
36-
return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd())
37-
}
38-
3925
// NewColorable returns an output stream that handles ANSI color sequences on Windows
40-
func NewColorable(f *os.File) io.Writer {
41-
return colorable.NewColorable(f)
26+
func NewColorable(w io.Writer) io.Writer {
27+
if f, isFile := w.(*os.File); isFile {
28+
return colorable.NewColorable(f)
29+
}
30+
return w
4231
}
4332

4433
func makeColorFunc(color string) func(string) string {

0 commit comments

Comments
 (0)
X Tutup