Skip to content

Commit

Permalink
Resolve PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
heaths committed Oct 18, 2023
1 parent 50f6653 commit 9ece9fc
Show file tree
Hide file tree
Showing 38 changed files with 123 additions and 64 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ issues:
- path: _test\.go
linters:
- staticcheck
text: "SA1019: tableprinter.NoHeaders is deprecated: use WithHeaders unless required otherwise."
text: "SA1019: tableprinter.NoHeader is deprecated: use WithHeader unless required otherwise."
19 changes: 14 additions & 5 deletions internal/tableprinter/table_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,25 @@ func NewWithWriter(w io.Writer, isTTY bool, maxWidth int, cs *iostreams.ColorSch
return tp
}

// WithHeaders defines the column names for a table.
// WithHeader defines the column names for a table.
// Panics if columns is nil or empty.
func WithHeaders(columns ...string) headerOption {
func WithHeader(columns ...string) headerOption {
if len(columns) == 0 {
panic("must define headers")
panic("must define header columns")
}
return headerOption{columns}
}

// NoHeader disable printing or checking for a table header.
//
// Deprecated: use WithHeaders unless required otherwise.
var NoHeaders = headerOption{}
// Deprecated: use WithHeader unless required otherwise.
var NoHeader = headerOption{}

// TruncateNonURL truncates any text that does not begin with "https://" or "http://".
// This is provided for backward compatibility with the old table printer for existing tables.
func TruncateNonURL(maxWidth int, s string) string {
if strings.HasPrefix(s, "https://") || strings.HasPrefix(s, "http://") {
return s
}
return text.Truncate(maxWidth, s)
}
47 changes: 47 additions & 0 deletions internal/tableprinter/table_printer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package tableprinter

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestTruncateNonURL(t *testing.T) {
tests := []struct {
name string
input string
want string
}{
{
name: "http url",
input: "http://github.com",
want: "http://github.com",
},
{
name: "https url",
input: "https://github.com",
want: "https://github.com",
},
{
name: "https url",
input: "https://github.com",
want: "https://github.com",
},
{
name: "git url",
input: "git@github.com",
want: "git@git...",
},
{
name: "non-url",
input: "just some plain text",
want: "just so...",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := TruncateNonURL(10, tt.input)
assert.Equal(t, tt.want, got)
})
}
}
2 changes: 1 addition & 1 deletion pkg/cmd/cache/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func listRun(opts *ListOptions) error {
opts.Now = time.Now()
}

tp := tableprinter.New(opts.IO, tableprinter.WithHeaders("ID", "KEY", "SIZE", "CREATED", "ACCESSED"))
tp := tableprinter.New(opts.IO, tableprinter.WithHeader("ID", "KEY", "SIZE", "CREATED", "ACCESSED"))
for _, cache := range result.ActionsCaches {
tp.AddField(opts.IO.ColorScheme().Cyan(fmt.Sprintf("%d", cache.Id)))
tp.AddField(cache.Key)
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/codespace/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (a *App) List(ctx context.Context, opts *listOptions, exporter cmdutil.Expo
if hasNonProdVSCSTarget {
headers = append(headers, "VSCS TARGET")
}
tp := tableprinter.New(a.io, tableprinter.WithHeaders(headers...))
tp := tableprinter.New(a.io, tableprinter.WithHeader(headers...))

cs := a.io.ColorScheme()
for _, apiCodespace := range codespaces {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/codespace/ports.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (a *App) ListPorts(ctx context.Context, selector *CodespaceSelector, export
}

cs := a.io.ColorScheme()
tp := tableprinter.New(a.io, tableprinter.WithHeaders("LABEL", "PORT", "VISIBILITY", "BROWSE URL"))
tp := tableprinter.New(a.io, tableprinter.WithHeader("LABEL", "PORT", "VISIBILITY", "BROWSE URL"))

for _, port := range portInfos {
// Convert the ACE to a friendly visibility string (private, org, public)
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/codespace/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (a *App) ViewCodespace(ctx context.Context, opts *viewOptions) error {
}

//nolint:staticcheck // SA1019: Showing NAME|VALUE headers adds nothing to table.
tp := tableprinter.New(a.io, tableprinter.NoHeaders)
tp := tableprinter.New(a.io, tableprinter.NoHeader)
c := codespace{selectedCodespace}
formattedName := formatNameForVSCSTarget(c.Name, c.VSCSTarget)

Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/extension/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
return false
}

tp := tableprinter.New(io, tableprinter.WithHeaders("", "REPO", "DESCRIPTION"))
tp := tableprinter.New(io, tableprinter.WithHeader("", "REPO", "DESCRIPTION"))
for _, repo := range result.Items {
if !strings.HasPrefix(repo.Name, "gh-") {
continue
Expand Down Expand Up @@ -265,7 +265,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
return cmdutil.NewNoResultsError("no installed extensions found")
}
cs := io.ColorScheme()
t := tableprinter.New(io, tableprinter.WithHeaders("NAME", "REPO", "VERSION"))
t := tableprinter.New(io, tableprinter.WithHeader("NAME", "REPO", "VERSION"))
for _, c := range cmds {
// TODO consider a Repo() on Extension interface
var repo string
Expand Down
8 changes: 6 additions & 2 deletions pkg/cmd/gist/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func listRun(opts *ListOptions) error {
}

cs := opts.IO.ColorScheme()
tp := tableprinter.New(opts.IO, tableprinter.WithHeaders("ID", "DESCRIPTION", "FILES", "", "UPDATED"))
tp := tableprinter.New(opts.IO, tableprinter.WithHeader("ID", "DESCRIPTION", "FILES", "", "UPDATED"))

for _, gist := range gists {
fileCount := len(gist.Files)
Expand All @@ -117,7 +117,11 @@ func listRun(opts *ListOptions) error {
}

tp.AddField(gist.ID)
tp.AddField(text.RemoveExcessiveWhitespace(description), tableprinter.WithColor(cs.Bold))
tp.AddField(
text.RemoveExcessiveWhitespace(description),
tableprinter.WithColor(cs.Bold),
tableprinter.WithTruncate(tableprinter.TruncateNonURL),
)
tp.AddField(text.Pluralize(fileCount, "file"))
tp.AddField(visibility, tableprinter.WithColor(visColor))
tp.AddTimeField(time.Now(), gist.UpdatedAt, cs.Gray)
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/gpg-key/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func listRun(opts *ListOptions) error {
return cmdutil.NewNoResultsError("no GPG keys present in the GitHub account")
}

t := tableprinter.New(opts.IO, tableprinter.WithHeaders("EMAIL", "KEY ID", "PUBLIC KEY", "ADDED", "EXPIRES"))
t := tableprinter.New(opts.IO, tableprinter.WithHeader("EMAIL", "KEY ID", "PUBLIC KEY", "ADDED", "EXPIRES"))
cs := opts.IO.ColorScheme()
now := time.Now()

Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/issue/develop/develop.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func developRunList(opts *DevelopOptions, apiClient *api.Client, issueRepo ghrep

func printLinkedBranches(io *iostreams.IOStreams, branches []api.LinkedBranch) {
cs := io.ColorScheme()
table := tableprinter.New(io, tableprinter.WithHeaders("BRANCH", "URL"))
table := tableprinter.New(io, tableprinter.WithHeader("BRANCH", "URL"))
for _, branch := range branches {
table.AddField(branch.BranchName, tableprinter.WithColor(cs.ColorFromString("cyan")))
table.AddField(branch.URL)
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/issue/list/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestIssueList_tty(t *testing.T) {
Showing 3 of 3 open issues in OWNER/REPO
TITLE LABELS UPDATED
ID TITLE LABELS UPDATED
#1 number won label about 1 day ago
#2 number too label about 1 month ago
#4 number fore label about 2 years ago
Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/issue/shared/display.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
func PrintIssues(io *iostreams.IOStreams, now time.Time, prefix string, totalCount int, issues []api.Issue) {
cs := io.ColorScheme()
isTTY := io.IsStdoutTTY()
headers := []string{""} // NUMBER, but often wider than issue numbers.
headers := []string{"ID"}
if !isTTY {
headers = append(headers, "STATE")
}
Expand All @@ -25,7 +25,7 @@ func PrintIssues(io *iostreams.IOStreams, now time.Time, prefix string, totalCou
"LABELS",
"UPDATED",
)
table := tableprinter.New(io, tableprinter.WithHeaders(headers...))
table := tableprinter.New(io, tableprinter.WithHeader(headers...))
for _, issue := range issues {
issueNum := strconv.Itoa(issue.Number)
if isTTY {
Expand All @@ -36,7 +36,7 @@ func PrintIssues(io *iostreams.IOStreams, now time.Time, prefix string, totalCou
if !isTTY {
table.AddField(issue.State)
}
table.AddField(text.RemoveExcessiveWhitespace(issue.Title))
table.AddField(text.RemoveExcessiveWhitespace(issue.Title), tableprinter.WithTruncate(tableprinter.TruncateNonURL))
table.AddField(issueLabelList(&issue, cs, isTTY))
table.AddTimeField(now, issue.UpdatedAt, cs.Gray)
table.EndRow()
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/label/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,11 @@ func listRun(opts *listOptions) error {

func printLabels(io *iostreams.IOStreams, labels []label) error {
cs := io.ColorScheme()
table := tableprinter.New(io, tableprinter.WithHeaders("NAME", "DESCRIPTION", "COLOR"))
table := tableprinter.New(io, tableprinter.WithHeader("NAME", "DESCRIPTION", "COLOR"))

for _, label := range labels {
table.AddField(label.Name, tableprinter.WithColor(cs.ColorFromRGB(label.Color)))
table.AddField(label.Description, tableprinter.WithTruncate(text.Truncate))
table.AddField(label.Description, tableprinter.WithTruncate(tableprinter.TruncateNonURL))
table.AddField("#" + label.Color)

table.EndRow()
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/pr/checks/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func printTable(io *iostreams.IOStreams, checks []check) error {
headers = []string{"NAME", "STATUS", "ELAPSED", "URL", "DESCRIPTION"}
}

tp := tableprinter.New(io, tableprinter.WithHeaders(headers...))
tp := tableprinter.New(io, tableprinter.WithHeader(headers...))

sort.Slice(checks, func(i, j int) bool {
b0 := checks[i].Bucket
Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/pr/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ func listRun(opts *ListOptions) error {
isTTY := opts.IO.IsStdoutTTY()

headers := []string{
"",
"ID",
"TITLE",
"BRANCH",
}
Expand All @@ -211,15 +211,15 @@ func listRun(opts *ListOptions) error {
}
headers = append(headers, "CREATED AT")

table := tableprinter.New(opts.IO, tableprinter.WithHeaders(headers...))
table := tableprinter.New(opts.IO, tableprinter.WithHeader(headers...))
for _, pr := range listResult.PullRequests {
prNum := strconv.Itoa(pr.Number)
if isTTY {
prNum = "#" + prNum
}

table.AddField(prNum, tableprinter.WithColor(cs.ColorFromString(shared.ColorForPRState(pr))))
table.AddField(text.RemoveExcessiveWhitespace(pr.Title))
table.AddField(text.RemoveExcessiveWhitespace(pr.Title), tableprinter.WithTruncate(tableprinter.TruncateNonURL))
table.AddField(pr.HeadLabel(), tableprinter.WithColor(cs.Cyan))
if !isTTY {
table.AddField(prStateWithDraft(&pr))
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/pr/list/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestPRList(t *testing.T) {
Showing 3 of 3 open pull requests in OWNER/REPO
TITLE BRANCH CREATED AT
ID TITLE BRANCH CREATED AT
#32 New feature feature about 3 hours ago
#29 Fixed bad bug hubot:bug-fix about 1 month ago
#28 Improve documentation docs about 2 years ago
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/project/field-list/field_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(config listConfig) error) *cobra.C
opts.number = int32(num)
}

t := tableprinter.New(f.IOStreams, tableprinter.WithHeaders("Name", "Data type", "ID"))
t := tableprinter.New(f.IOStreams, tableprinter.WithHeader("Name", "Data type", "ID"))
config := listConfig{
io: f.IOStreams,
tp: t,
Expand Down
8 changes: 4 additions & 4 deletions pkg/cmd/project/field-list/field_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func TestRunList_User(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
tp: tableprinter.New(ios, tableprinter.NoHeaders),
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
owner: "monalisa",
Expand Down Expand Up @@ -256,7 +256,7 @@ func TestRunList_Org(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
tp: tableprinter.New(ios, tableprinter.NoHeaders),
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
owner: "github",
Expand Down Expand Up @@ -339,7 +339,7 @@ func TestRunList_Me(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
tp: tableprinter.New(ios, tableprinter.NoHeaders),
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
owner: "@me",
Expand Down Expand Up @@ -406,7 +406,7 @@ func TestRunList_Empty(t *testing.T) {

ios, _, _, _ := iostreams.Test()
config := listConfig{
tp: tableprinter.New(ios, tableprinter.NoHeaders),
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
owner: "@me",
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/project/item-list/item_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(config listConfig) error) *cobra.C
opts.number = int32(num)
}

t := tableprinter.New(f.IOStreams, tableprinter.WithHeaders("Type", "Title", "Number", "Repository", "ID"))
t := tableprinter.New(f.IOStreams, tableprinter.WithHeader("Type", "Title", "Number", "Repository", "ID"))
config := listConfig{
io: f.IOStreams,
tp: t,
Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/project/item-list/item_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func TestRunList_User(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
tp: tableprinter.New(ios, tableprinter.NoHeaders),
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
owner: "monalisa",
Expand Down Expand Up @@ -285,7 +285,7 @@ func TestRunList_Org(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
tp: tableprinter.New(ios, tableprinter.NoHeaders),
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
owner: "github",
Expand Down Expand Up @@ -382,7 +382,7 @@ func TestRunList_Me(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
tp: tableprinter.New(ios, tableprinter.NoHeaders),
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
owner: "@me",
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/project/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(config listConfig) error) *cobra.C
URLOpener := func(url string) error {
return f.Browser.Browse(url)
}
t := tableprinter.New(f.IOStreams, tableprinter.WithHeaders("Number", "Title", "State", "ID"))
t := tableprinter.New(f.IOStreams, tableprinter.WithHeader("Number", "Title", "State", "ID"))
config := listConfig{
tp: t,
client: client,
Expand Down
Loading

0 comments on commit 9ece9fc

Please sign in to comment.