Better enforce header option at compile time

Require headerOption when constructing tableprinter.TablePrinter. Panic if headers are nil or empty - expected to be caught in tests.

.golangci.yml

@@ -6,3 +6,8 @@ linters:
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0
+  exclude-rules:
+  - path: _test\.go
+    linters:
+    - staticcheck
+    text: "SA1019: tableprinter.NoHeaders is deprecated: use WithHeaders unless required otherwise."

internal/tableprinter/table_printer.go

@@ -14,37 +14,21 @@ type TablePrinter struct {
 	tableprinter.TablePrinter
 	isTTY bool
 	cs    *iostreams.ColorScheme
-
-	// Assert that we tried to add a header.
-	addedHeader bool
 }
 
+// IsTTY gets wether the TablePrinter will render to a terminal.
 func (t *TablePrinter) IsTTY() bool {
 	return t.isTTY
 }
 
-func (t *TablePrinter) HeaderRow(columns ...string) {
-	if t.addedHeader {
-		return
-	}
-	t.addedHeader = true
-
-	if !t.isTTY {
-		return
-	}
-	for _, col := range columns {
-		// TODO: Consider truncating longer headers e.g., NUMBER, or removing unnecessary headers e.g., DESCRIPTION with no descriptions.
-		t.AddField(strings.ToUpper(col), WithColor(t.cs.Bold))
-	}
-	t.EndRow()
-}
-
-// In tty mode display the fuzzy time difference between now and t.
-// In nontty mode just display t with the time.RFC3339 format.
+// AddTimeField in TTY mode displays the fuzzy time difference between now and t.
+// In non-TTY mode it just displays t with the time.RFC3339 format.
 func (tp *TablePrinter) AddTimeField(now, t time.Time, c func(string) string) {
-	tf := t.Format(time.RFC3339)
+	var tf string
 	if tp.isTTY {
 		tf = text.FuzzyAgo(now, t)
+	} else {
+		tf = t.Format(time.RFC3339)
 	}
 	tp.AddField(tf, tableprinter.WithColor(c))
 }
@@ -54,39 +38,50 @@ var (
 	WithColor    = tableprinter.WithColor
 )
 
-type option func(*TablePrinter)
+type headerOption struct {
+	columns []string
+}
 
-func New(ios *iostreams.IOStreams, options ...option) *TablePrinter {
+// New creates a TablePrinter from an IOStreams.
+func New(ios *iostreams.IOStreams, headers headerOption) *TablePrinter {
 	maxWidth := 80
 	isTTY := ios.IsStdoutTTY()
 	if isTTY {
 		maxWidth = ios.TerminalWidth()
 	}
 
-	return NewWithOptions(ios.Out, isTTY, maxWidth, ios.ColorScheme(), options...)
+	return NewWithWriter(ios.Out, isTTY, maxWidth, ios.ColorScheme(), headers)
 }
 
-func NewWithOptions(w io.Writer, isTTY bool, maxWidth int, cs *iostreams.ColorScheme, options ...option) *TablePrinter {
+// NewWithWriter creates a TablePrinter from a Writer, whether the output is a terminal, the terminal width, and more.
+func NewWithWriter(w io.Writer, isTTY bool, maxWidth int, cs *iostreams.ColorScheme, headers headerOption) *TablePrinter {
 	tp := &TablePrinter{
 		TablePrinter: tableprinter.New(w, isTTY, maxWidth),
 		isTTY:        isTTY,
 		cs:           cs,
 	}
 
-	for _, opt := range options {
-		opt(tp)
+	if isTTY && len(headers.columns) > 0 {
+		for _, header := range headers.columns {
+			// TODO: Consider truncating longer headers e.g., NUMBER, or removing unnecessary headers e.g., DESCRIPTION with no descriptions.
+			tp.AddField(strings.ToUpper(header), WithColor(cs.Bold))
+		}
+		tp.EndRow()
 	}
-	return tp
-}
 
-// NoHeader disable printing or checking for a table header.
-func NoHeader(tp *TablePrinter) {
-	tp.addedHeader = true
+	return tp
 }
 
-func (t *TablePrinter) Render() error {
-	if !t.addedHeader {
-		panic("must call HeaderRow")
+// WithHeaders defines the column names for a table.
+// Panics if columns is nil or empty.
+func WithHeaders(columns ...string) headerOption {
+	if len(columns) == 0 {
+		panic("must define headers")
 	}
-	return t.TablePrinter.Render()
+	return headerOption{columns}
 }
+
+// NoHeader disable printing or checking for a table header.
+//
+// Deprecated: use WithHeaders unless required otherwise.
+var NoHeaders = headerOption{}

pkg/cmd/cache/list/list.go

@@ -115,8 +115,7 @@ func listRun(opts *ListOptions) error {
 		opts.Now = time.Now()
 	}
 
-	tp := tableprinter.New(opts.IO)
-	tp.HeaderRow("ID", "KEY", "SIZE", "CREATED", "ACCESSED")
+	tp := tableprinter.New(opts.IO, tableprinter.WithHeaders("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)

pkg/cmd/codespace/list.go

@@ -116,8 +116,6 @@ func (a *App) List(ctx context.Context, opts *listOptions, exporter cmdutil.Expo
 		return a.browser.Browse(fmt.Sprintf("%s/codespaces?repository_id=%d", a.apiClient.ServerURL(), codespaces[0].Repository.ID))
 	}
 
-	tp := tableprinter.New(a.io)
-
 	headers := []string{
 		"NAME",
 		"DISPLAY NAME",
@@ -134,7 +132,7 @@ func (a *App) List(ctx context.Context, opts *listOptions, exporter cmdutil.Expo
 	if hasNonProdVSCSTarget {
 		headers = append(headers, "VSCS TARGET")
 	}
-	tp.HeaderRow(headers...)
+	tp := tableprinter.New(a.io, tableprinter.WithHeaders(headers...))
 
 	cs := a.io.ColorScheme()
 	for _, apiCodespace := range codespaces {

pkg/cmd/codespace/ports.go

@@ -107,8 +107,7 @@ func (a *App) ListPorts(ctx context.Context, selector *CodespaceSelector, export
 	}
 
 	cs := a.io.ColorScheme()
-	tp := tableprinter.New(a.io)
-	tp.HeaderRow("LABEL", "PORT", "VISIBILITY", "BROWSE URL")
+	tp := tableprinter.New(a.io, tableprinter.WithHeaders("LABEL", "PORT", "VISIBILITY", "BROWSE URL"))
 
 	for _, port := range portInfos {
 		// Convert the ACE to a friendly visibility string (private, org, public)

pkg/cmd/codespace/view.go

@@ -73,7 +73,8 @@ func (a *App) ViewCodespace(ctx context.Context, opts *viewOptions) error {
 		return opts.exporter.Write(a.io, selectedCodespace)
 	}
 
-	tp := tableprinter.New(a.io, tableprinter.NoHeader)
+	//nolint:staticcheck // SA1019: Showing NAME|VALUE headers adds nothing to table.
+	tp := tableprinter.New(a.io, tableprinter.NoHeaders)
 	c := codespace{selectedCodespace}
 	formattedName := formatNameForVSCSTarget(c.Name, c.VSCSTarget)
 

pkg/cmd/extension/command.go

@@ -214,9 +214,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
 						return false
 					}
 
-					tp := tableprinter.New(io)
-					tp.HeaderRow("", "REPO", "DESCRIPTION")
-
+					tp := tableprinter.New(io, tableprinter.WithHeaders("", "REPO", "DESCRIPTION"))
 					for _, repo := range result.Items {
 						if !strings.HasPrefix(repo.Name, "gh-") {
 							continue
@@ -267,8 +265,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
 					return cmdutil.NewNoResultsError("no installed extensions found")
 				}
 				cs := io.ColorScheme()
-				t := tableprinter.New(io)
-				t.HeaderRow("NAME", "REPO", "VERSION")
+				t := tableprinter.New(io, tableprinter.WithHeaders("NAME", "REPO", "VERSION"))
 				for _, c := range cmds {
 					// TODO consider a Repo() on Extension interface
 					var repo string

pkg/cmd/gist/list/list.go

@@ -94,9 +94,7 @@ func listRun(opts *ListOptions) error {
 	}
 
 	cs := opts.IO.ColorScheme()
-
-	tp := tableprinter.New(opts.IO)
-	tp.HeaderRow("ID", "DESCRIPTION", "FILES", "", "UPDATED")
+	tp := tableprinter.New(opts.IO, tableprinter.WithHeaders("ID", "DESCRIPTION", "FILES", "", "UPDATED"))
 
 	for _, gist := range gists {
 		fileCount := len(gist.Files)

pkg/cmd/gpg-key/list/list.go

@@ -71,12 +71,10 @@ func listRun(opts *ListOptions) error {
 		return cmdutil.NewNoResultsError("no GPG keys present in the GitHub account")
 	}
 
-	t := tableprinter.New(opts.IO)
+	t := tableprinter.New(opts.IO, tableprinter.WithHeaders("EMAIL", "KEY ID", "PUBLIC KEY", "ADDED", "EXPIRES"))
 	cs := opts.IO.ColorScheme()
 	now := time.Now()
 
-	t.HeaderRow("EMAIL", "KEY ID", "PUBLIC KEY", "ADDED", "EXPIRES")
-
 	for _, gpgKey := range gpgKeys {
 		t.AddField(gpgKey.Emails.String())
 		t.AddField(gpgKey.KeyID)

pkg/cmd/issue/develop/develop.go

@@ -199,8 +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)
-	table.HeaderRow("BRANCH", "URL")
+	table := tableprinter.New(io, tableprinter.WithHeaders("BRANCH", "URL"))
 	for _, branch := range branches {
 		table.AddField(branch.BranchName, tableprinter.WithColor(cs.ColorFromString("cyan")))
 		table.AddField(branch.URL)

pkg/cmd/issue/shared/display.go

@@ -15,29 +15,29 @@ import (
 
 func PrintIssues(io *iostreams.IOStreams, now time.Time, prefix string, totalCount int, issues []api.Issue) {
 	cs := io.ColorScheme()
-	table := tableprinter.New(io)
+	isTTY := io.IsStdoutTTY()
 	headers := []string{""} // NUMBER, but often wider than issue numbers.
-	if !table.IsTTY() {
+	if !isTTY {
 		headers = append(headers, "STATE")
 	}
 	headers = append(headers,
 		"TITLE",
 		"LABELS",
 		"UPDATED",
 	)
-	table.HeaderRow(headers...)
+	table := tableprinter.New(io, tableprinter.WithHeaders(headers...))
 	for _, issue := range issues {
 		issueNum := strconv.Itoa(issue.Number)
-		if table.IsTTY() {
+		if isTTY {
 			issueNum = "#" + issueNum
 		}
 		issueNum = prefix + issueNum
 		table.AddField(issueNum, tableprinter.WithColor(cs.ColorFromString(prShared.ColorForIssueState(issue))))
-		if !table.IsTTY() {
+		if !isTTY {
 			table.AddField(issue.State)
 		}
 		table.AddField(text.RemoveExcessiveWhitespace(issue.Title))
-		table.AddField(issueLabelList(&issue, cs, table.IsTTY()))
+		table.AddField(issueLabelList(&issue, cs, isTTY))
 		table.AddTimeField(now, issue.UpdatedAt, cs.Gray)
 		table.EndRow()
 	}

pkg/cmd/label/list.go

@@ -134,8 +134,7 @@ func listRun(opts *listOptions) error {
 
 func printLabels(io *iostreams.IOStreams, labels []label) error {
 	cs := io.ColorScheme()
-	table := tableprinter.New(io)
-	table.HeaderRow("NAME", "DESCRIPTION", "COLOR")
+	table := tableprinter.New(io, tableprinter.WithHeaders("NAME", "DESCRIPTION", "COLOR"))
 
 	for _, label := range labels {
 		table.AddField(label.Name, tableprinter.WithColor(cs.ColorFromRGB(label.Color)))

pkg/cmd/pr/checks/output.go

@@ -92,7 +92,14 @@ func printSummary(io *iostreams.IOStreams, counts checkCounts) {
 }
 
 func printTable(io *iostreams.IOStreams, checks []check) error {
-	tp := tableprinter.New(io)
+	var headers []string
+	if io.IsStdoutTTY() {
+		headers = []string{"", "NAME", "DESCRIPTION", "ELAPSED", "URL"}
+	} else {
+		headers = []string{"NAME", "STATUS", "ELAPSED", "URL", "DESCRIPTION"}
+	}
+
+	tp := tableprinter.New(io, tableprinter.WithHeaders(headers...))
 
 	sort.Slice(checks, func(i, j int) bool {
 		b0 := checks[i].Bucket
@@ -112,12 +119,6 @@ func printTable(io *iostreams.IOStreams, checks []check) error {
 		return (b0 == "fail") || (b0 == "pending" && b1 == "success")
 	})
 
-	if io.IsStdoutTTY() {
-		tp.HeaderRow("", "NAME", "DESCRIPTION", "ELAPSED", "URL")
-	} else {
-		tp.HeaderRow("NAME", "STATUS", "ELAPSED", "URL", "DESCRIPTION")
-	}
-
 	for _, o := range checks {
 		addRow(tp, io, o)
 	}

pkg/cmd/pr/list/list.go

@@ -199,32 +199,32 @@ func listRun(opts *ListOptions) error {
 	}
 
 	cs := opts.IO.ColorScheme()
-	table := tableprinter.New(opts.IO)
+	isTTY := opts.IO.IsStdoutTTY()
 
 	headers := []string{
 		"",
 		"TITLE",
 		"BRANCH",
 	}
-	if !table.IsTTY() {
+	if !isTTY {
 		headers = append(headers, "STATE")
 	}
 	headers = append(headers, "CREATED AT")
-	table.HeaderRow(headers...)
 
+	table := tableprinter.New(opts.IO, tableprinter.WithHeaders(headers...))
 	for _, pr := range listResult.PullRequests {
 		prNum := strconv.Itoa(pr.Number)
-		if table.IsTTY() {
+		if isTTY {
 			prNum = "#" + prNum
 		}
 
 		table.AddField(prNum, tableprinter.WithColor(cs.ColorFromString(shared.ColorForPRState(pr))))
 		table.AddField(text.RemoveExcessiveWhitespace(pr.Title))
 		table.AddField(pr.HeadLabel(), tableprinter.WithColor(cs.Cyan))
-		if !table.IsTTY() {
+		if !isTTY {
 			table.AddField(prStateWithDraft(&pr))
 		}
-		if table.IsTTY() {
+		if isTTY {
 			table.AddField(text.FuzzyAgo(opts.Now(), pr.CreatedAt), tableprinter.WithColor(cs.Gray))
 		} else {
 			table.AddField(pr.CreatedAt.String())

pkg/cmd/project/field-list/field_list.go

@@ -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)
+			t := tableprinter.New(f.IOStreams, tableprinter.WithHeaders("Name", "Data type", "ID"))
 			config := listConfig{
 				io:     f.IOStreams,
 				tp:     t,
@@ -109,8 +109,6 @@ func printResults(config listConfig, fields []queries.ProjectField, login string
 		return cmdutil.NewNoResultsError(fmt.Sprintf("Project %d for owner %s has no fields", config.opts.number, login))
 	}
 
-	config.tp.HeaderRow("Name", "Data type", "ID")
-
 	for _, f := range fields {
 		config.tp.AddField(f.Name())
 		config.tp.AddField(f.Type())

pkg/cmd/project/field-list/field_list_test.go

@@ -163,7 +163,7 @@ func TestRunList_User(t *testing.T) {
 
 	ios, _, stdout, _ := iostreams.Test()
 	config := listConfig{
-		tp: tableprinter.New(ios),
+		tp: tableprinter.New(ios, tableprinter.NoHeaders),
 		opts: listOpts{
 			number: 1,
 			owner:  "monalisa",
@@ -256,7 +256,7 @@ func TestRunList_Org(t *testing.T) {
 
 	ios, _, stdout, _ := iostreams.Test()
 	config := listConfig{
-		tp: tableprinter.New(ios),
+		tp: tableprinter.New(ios, tableprinter.NoHeaders),
 		opts: listOpts{
 			number: 1,
 			owner:  "github",
@@ -339,7 +339,7 @@ func TestRunList_Me(t *testing.T) {
 
 	ios, _, stdout, _ := iostreams.Test()
 	config := listConfig{
-		tp: tableprinter.New(ios),
+		tp: tableprinter.New(ios, tableprinter.NoHeaders),
 		opts: listOpts{
 			number: 1,
 			owner:  "@me",
@@ -406,7 +406,7 @@ func TestRunList_Empty(t *testing.T) {
 
 	ios, _, _, _ := iostreams.Test()
 	config := listConfig{
-		tp: tableprinter.New(ios),
+		tp: tableprinter.New(ios, tableprinter.NoHeaders),
 		opts: listOpts{
 			number: 1,
 			owner:  "@me",