STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #1343 from cli/improve-issue

Browse source 
issue scriptability improvements
This commit is contained in:
Nate Smith 2020-07-14 12:45:03 -05:00 • committed by GitHub
commit 2b96d2ce1f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 271 additions and 57 deletions
Download patch file Download diff file Expand all files Collapse all files

48
command/issue.go
View file

@ -184,8 +184,9 @@ func issueList(cmd *cobra.Command, args []string) error {
})
title := listHeader(ghrepo.FullName(baseRepo), "issue", len(listResult.Issues), listResult.TotalCount, hasFilters)
// TODO: avoid printing header if piped to a script
fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title)
if connectedToTerminal(cmd) {
fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title)
}
out := cmd.OutOrStdout()
@ -278,8 +279,11 @@ func issueView(cmd *cobra.Command, args []string) error {
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
return utils.OpenInBrowser(openURL)
}
out := colorableOut(cmd)
return printIssuePreview(out, issue)
if connectedToTerminal(cmd) {
return printHumanIssuePreview(colorableOut(cmd), issue)
}
return printRawIssuePreview(cmd.OutOrStdout(), issue)
}
func issueStateTitleWithColor(state string) string {
@ -306,7 +310,28 @@ func listHeader(repoName string, itemName string, matchCount int, totalMatchCoun
return fmt.Sprintf("Showing %d of %s in %s", matchCount, utils.Pluralize(totalMatchCount, itemName), repoName)
}
func printIssuePreview(out io.Writer, issue *api.Issue) error {
func printRawIssuePreview(out io.Writer, issue *api.Issue) error {
assignees := issueAssigneeList(*issue)
labels := issueLabelList(*issue)
projects := issueProjectList(*issue)
// Print empty strings for empty values so the number of metadata lines is consistent when
// processing many issues with head and grep.
fmt.Fprintf(out, "title:\t%s\n", issue.Title)
fmt.Fprintf(out, "state:\t%s\n", issue.State)
fmt.Fprintf(out, "author:\t%s\n", issue.Author.Login)
fmt.Fprintf(out, "labels:\t%s\n", labels)
fmt.Fprintf(out, "comments:\t%d\n", issue.Comments.TotalCount)
fmt.Fprintf(out, "assignees:\t%s\n", assignees)
fmt.Fprintf(out, "projects:\t%s\n", projects)
fmt.Fprintf(out, "milestone:\t%s\n", issue.Milestone.Title)
fmt.Fprintln(out, "--")
fmt.Fprintln(out, issue.Body)
return nil
}
func printHumanIssuePreview(out io.Writer, issue *api.Issue) error {
now := time.Now()
ago := now.Sub(issue.CreatedAt)
@ -464,6 +489,10 @@ func issueCreate(cmd *cobra.Command, args []string) error {
interactive := !(cmd.Flags().Changed("title") && cmd.Flags().Changed("body"))
if interactive && !connectedToTerminal(cmd) {
return fmt.Errorf("must provide --title and --body when not attached to a terminal")
}
if interactive {
var legacyTemplateFile *string
if baseOverride == "" {
@ -625,9 +654,16 @@ func printIssues(w io.Writer, prefix string, totalCount int, issues []api.Issue)
now := time.Now()
ago := now.Sub(issue.UpdatedAt)
table.AddField(issueNum, nil, colorFuncForState(issue.State))
if !table.IsTTY() {
table.AddField(issue.State, nil, nil)
}
table.AddField(replaceExcessiveWhitespace(issue.Title), nil, nil)
table.AddField(labels, nil, utils.Gray)
table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray)
if table.IsTTY() {
table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray)
} else {
table.AddField(issue.UpdatedAt.String(), nil, nil)
}
table.EndRow()
}
_ = table.Render()

152
command/issue_test.go
View file

@ -18,6 +18,7 @@ import (
func TestIssueStatus(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
@ -107,8 +108,31 @@ func TestIssueStatus_disabledIssues(t *testing.T) {
}
}
func TestIssueList(t *testing.T) {
func TestIssueList_nontty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(false)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
httpmock.GraphQL(`query IssueList\b`),
httpmock.FileResponse("../test/fixtures/issueList.json"))
output, err := RunCommand("issue list")
if err != nil {
t.Errorf("error running command `issue list`: %v", err)
}
eq(t, output.Stderr(), "")
test.ExpectLines(t, output.String(),
`1[\t]+number won[\t]+label[\t]+\d+`,
`2[\t]+number too[\t]+label[\t]+\d+`,
`4[\t]+number fore[\t]+label[\t]+\d+`)
}
func TestIssueList_tty(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
defer stubTerminal(true)()
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(
@ -125,21 +149,14 @@ Showing 3 of 3 issues in OWNER/REPO
`)
expectedIssues := []*regexp.Regexp{
regexp.MustCompile(`(?m)^1\t.*won`),
regexp.MustCompile(`(?m)^2\t.*too`),
regexp.MustCompile(`(?m)^4\t.*fore`),
}
for _, r := range expectedIssues {
if !r.MatchString(output.String()) {
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
return
}
}
test.ExpectLines(t, output.String(),
"number won",
"number too",
"number fore")
}
func TestIssueList_withFlags(t *testing.T) {
func TestIssueList_tty_withFlags(t *testing.T) {
defer stubTerminal(true)()
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
@ -296,7 +313,90 @@ func TestIssueView_web_numberArgWithHash(t *testing.T) {
eq(t, url, "https://github.com/OWNER/REPO/issues/123")
}
func TestIssueView_Preview(t *testing.T) {
func TestIssueView_nontty_Preview(t *testing.T) {
defer stubTerminal(false)()
tests := map[string]struct {
ownerRepo string
command string
fixture string
expectedOutputs []string
}{
"Open issue without metadata": {
ownerRepo: "master",
command: "issue view 123",
fixture: "../test/fixtures/issueView_preview.json",
expectedOutputs: []string{
`title:\tix of coins`,
`state:\tOPEN`,
`comments:\t9`,
`author:\tmarseilles`,
`assignees:`,
`\*\*bold story\*\*`,
},
},
"Open issue with metadata": {
ownerRepo: "master",
command: "issue view 123",
fixture: "../test/fixtures/issueView_previewWithMetadata.json",
expectedOutputs: []string{
`title:\tix of coins`,
`assignees:\tmarseilles, monaco`,
`author:\tmarseilles`,
`state:\tOPEN`,
`comments:\t9`,
`labels:\tone, two, three, four, five`,
`projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`milestone:\tuluru\n`,
`\*\*bold story\*\*`,
},
},
"Open issue with empty body": {
ownerRepo: "master",
command: "issue view 123",
fixture: "../test/fixtures/issueView_previewWithEmptyBody.json",
expectedOutputs: []string{
`title:\tix of coins`,
`state:\tOPEN`,
`author:\tmarseilles`,
`labels:\ttarot`,
},
},
"Closed issue": {
ownerRepo: "master",
command: "issue view 123",
fixture: "../test/fixtures/issueView_previewClosedState.json",
expectedOutputs: []string{
`title:\tix of coins`,
`state:\tCLOSED`,
`\*\*bold story\*\*`,
`author:\tmarseilles`,
`labels:\ttarot`,
`\*\*bold story\*\*`,
},
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
initBlankContext("", "OWNER/REPO", tc.ownerRepo)
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture))
output, err := RunCommand(tc.command)
if err != nil {
t.Errorf("error running command `%v`: %v", tc.command, err)
}
eq(t, output.Stderr(), "")
test.ExpectLines(t, output.String(), tc.expectedOutputs...)
})
}
}
func TestIssueView_tty_Preview(t *testing.T) {
defer stubTerminal(true)()
tests := map[string]struct {
ownerRepo string
command string
@ -448,6 +548,28 @@ func TestIssueView_web_urlArg(t *testing.T) {
eq(t, url, "https://github.com/OWNER/REPO/issues/123")
}
func TestIssueCreate_nontty_error(t *testing.T) {
defer stubTerminal(false)()
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()
http.StubRepoResponse("OWNER", "REPO")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"id": "REPOID",
"hasIssuesEnabled": true
} } }
`))
_, err := RunCommand(`issue create -t hello`)
if err == nil {
t.Fatal("expected error running command `issue create`")
}
assert.Equal(t, "must provide --title and --body when not attached to a terminal", err.Error())
}
func TestIssueCreate(t *testing.T) {
initBlankContext("", "OWNER/REPO", "master")
http := initFakeHTTP()

4
command/root.go
View file

@ -407,3 +407,7 @@ func ExpandAlias(args []string) ([]string, error) {
return args[1:], nil
}
func connectedToTerminal(cmd *cobra.Command) bool {
return utils.IsTerminal(cmd.InOrStdin()) && utils.IsTerminal(cmd.OutOrStdout())
}

24
command/testing.go
View file

@ -12,6 +12,7 @@ import (
"github.com/cli/cli/context"
"github.com/cli/cli/internal/config"
"github.com/cli/cli/pkg/httpmock"
"github.com/cli/cli/utils"
"github.com/google/shlex"
"github.com/spf13/pflag"
)
@ -169,3 +170,26 @@ func (s errorStub) Output() ([]byte, error) {
func (s errorStub) Run() error {
return errors.New(s.message)
}
func stubTerminal(connected bool) func() {
isTerminal := utils.IsTerminal
utils.IsTerminal = func(_ interface{}) bool {
return connected
}
terminalSize := utils.TerminalSize
if connected {
utils.TerminalSize = func(_ interface{}) (int, int, error) {
return 80, 20, nil
}
} else {
utils.TerminalSize = func(_ interface{}) (int, int, error) {
return 0, 0, fmt.Errorf("terminal connection stubbed to false")
}
}
return func() {
utils.IsTerminal = isTerminal
utils.TerminalSize = terminalSize
}
}

21
utils/color.go
View file

@ -5,7 +5,6 @@ import (
"os"
"github.com/mattn/go-colorable"
"github.com/mattn/go-isatty"
"github.com/mgutz/ansi"
)
@ -23,22 +22,12 @@ var (
Bold = makeColorFunc("default+b")
)
func isStdoutTerminal() bool {
if !checkedTerminal {
_isStdoutTerminal = IsTerminal(os.Stdout)
checkedTerminal = true
}
return _isStdoutTerminal
}
// IsTerminal reports whether the file descriptor is connected to a terminal
func IsTerminal(f *os.File) bool {
return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd())
}
// NewColorable returns an output stream that handles ANSI color sequences on Windows
func NewColorable(f *os.File) io.Writer {
return colorable.NewColorable(f)
func NewColorable(w io.Writer) io.Writer {
if f, isFile := w.(*os.File); isFile {
return colorable.NewColorable(f)
}
return w
}
func makeColorFunc(color string) func(string) string {

35
utils/table_printer.go
View file

@ -9,8 +9,6 @@ import (
"strings"
"github.com/cli/cli/pkg/text"
"github.com/mattn/go-isatty"
"golang.org/x/crypto/ssh/terminal"
)
type TablePrinter interface {
@ -21,26 +19,23 @@ type TablePrinter interface {
}
func NewTablePrinter(w io.Writer) TablePrinter {
if outFile, isFile := w.(*os.File); isFile {
// TODO: use utils.IsTerminal()
isCygwin := isatty.IsCygwinTerminal(outFile.Fd())
if isatty.IsTerminal(outFile.Fd()) || isCygwin {
ttyWidth := 80
if w, _, err := terminal.GetSize(int(outFile.Fd())); err == nil {
ttyWidth = w
} else if isCygwin {
tputCmd := exec.Command("tput", "cols")
tputCmd.Stdin = os.Stdin
if out, err := tputCmd.Output(); err == nil {
if w, err := strconv.Atoi(strings.TrimSpace(string(out))); err == nil {
ttyWidth = w
}
if IsTerminal(w) {
isCygwin := IsCygwinTerminal(w)
ttyWidth := 80
if termWidth, _, err := TerminalSize(w); err == nil {
ttyWidth = termWidth
} else if isCygwin {
tputCmd := exec.Command("tput", "cols")
tputCmd.Stdin = os.Stdin
if out, err := tputCmd.Output(); err == nil {
if w, err := strconv.Atoi(strings.TrimSpace(string(out))); err == nil {
ttyWidth = w
}
}
return &ttyTablePrinter{
out: NewColorable(outFile),
maxWidth: ttyWidth,
}
}
return &ttyTablePrinter{
out: NewColorable(w),
maxWidth: ttyWidth,
}
}
return &tsvTablePrinter{

44
utils/terminal.go Normal file
View file

@ -0,0 +1,44 @@
package utils
import (
"fmt"
"os"
"github.com/mattn/go-isatty"
"golang.org/x/crypto/ssh/terminal"
)
func isStdoutTerminal() bool {
if !checkedTerminal {
_isStdoutTerminal = IsTerminal(os.Stdout)
checkedTerminal = true
}
return _isStdoutTerminal
}
// TODO I don't like this use of interface{} but we need to accept both io.Writer and io.Reader
// interfaces.
var IsTerminal = func(w interface{}) bool {
if f, isFile := w.(*os.File); isFile {
return isatty.IsTerminal(f.Fd()) || IsCygwinTerminal(f)
}
return false
}
func IsCygwinTerminal(w interface{}) bool {
if f, isFile := w.(*os.File); isFile {
return isatty.IsCygwinTerminal(f.Fd())
}
return false
}
var TerminalSize = func(w interface{}) (int, int, error) {
if f, isFile := w.(*os.File); isFile {
return terminal.GetSize(int(f.Fd()))
}
return 0, 0, fmt.Errorf("%v is not a file", w)
}