Skip to content

Commit 2b96d2c

Browse files
author
Nate Smith
authored
Merge pull request cli#1343 from cli/improve-issue
issue scriptability improvements
2 parents 53ff384 + 754dcb7 commit 2b96d2c

File tree

7 files changed

+271
-57
lines changed

7 files changed

+271
-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 consistent when
319+
// processing many issues with head and grep.
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("must provide --title and --body when not attached to a terminal")
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(issue.UpdatedAt.String(), nil, nil)
666+
}
631667
table.EndRow()
632668
}
633669
_ = table.Render()

‎command/issue_test.go‎

Lines changed: 137 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+
http.Register(
118+
httpmock.GraphQL(`query IssueList\b`),
119+
httpmock.FileResponse("../test/fixtures/issueList.json"))
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,90 @@ 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+
http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture))
385+
386+
output, err := RunCommand(tc.command)
387+
if err != nil {
388+
t.Errorf("error running command `%v`: %v", tc.command, err)
389+
}
390+
391+
eq(t, output.Stderr(), "")
392+
393+
test.ExpectLines(t, output.String(), tc.expectedOutputs...)
394+
})
395+
}
396+
}
397+
398+
func TestIssueView_tty_Preview(t *testing.T) {
399+
defer stubTerminal(true)()
300400
tests := map[string]struct {
301401
ownerRepo string
302402
command string
@@ -448,6 +548,28 @@ func TestIssueView_web_urlArg(t *testing.T) {
448548
eq(t, url, "https://github.com/OWNER/REPO/issues/123")
449549
}
450550

551+
func TestIssueCreate_nontty_error(t *testing.T) {
552+
defer stubTerminal(false)()
553+
initBlankContext("", "OWNER/REPO", "master")
554+
http := initFakeHTTP()
555+
http.StubRepoResponse("OWNER", "REPO")
556+
557+
http.StubResponse(200, bytes.NewBufferString(`
558+
{ "data": { "repository": {
559+
"id": "REPOID",
560+
"hasIssuesEnabled": true
561+
} } }
562+
`))
563+
564+
_, err := RunCommand(`issue create -t hello`)
565+
if err == nil {
566+
t.Fatal("expected error running command `issue create`")
567+
}
568+
569+
assert.Equal(t, "must provide --title and --body when not attached to a terminal", err.Error())
570+
571+
}
572+
451573
func TestIssueCreate(t *testing.T) {
452574
initBlankContext("", "OWNER/REPO", "master")
453575
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)