Skip to content

Commit

Permalink
Some polish and style nits
Browse files Browse the repository at this point in the history
  • Loading branch information
samcoe committed Oct 19, 2023
1 parent 5a19a46 commit d727f0b
Show file tree
Hide file tree
Showing 15 changed files with 48 additions and 99 deletions.
5 changes: 0 additions & 5 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,3 @@ linters:
issues:
max-issues-per-linter: 0
max-same-issues: 0
exclude-rules:
- path: _test\.go
linters:
- staticcheck
text: "SA1019: tableprinter.NoHeader is deprecated: use WithHeader unless required otherwise."
24 changes: 5 additions & 19 deletions internal/tableprinter/table_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,26 +64,21 @@ func NewWithWriter(w io.Writer, isTTY bool, maxWidth int, cs *iostreams.ColorSch
if isTTY && len(headers.columns) > 0 {
// Make sure all headers are uppercase.
for i := range headers.columns {
// TODO: Consider truncating longer headers e.g., NUMBER, or removing unnecessary headers e.g., DESCRIPTION with no descriptions.
headers.columns[i] = strings.ToUpper(headers.columns[i])
}

// Make sure all header columns are padded - even the last one - to apply the proper TableHeader style.
// Checking cs.Enabled() avoids having to do that for nearly all CLI tests.
// Make sure all header columns are padded - even the last one. Previously, the last header column
// was not padded. In tests cs.Enabled() is false which allows us to avoid having to fix up
// numerous tests that verify header padding.
var paddingFunc func(int, string) string
if cs.Enabled() {
paddingFunc = func(width int, text string) string {
if l := len(text); l < width {
return text + strings.Repeat(" ", width-l)
}
return text
}
paddingFunc = text.PadRight
}

tp.AddHeader(
headers.columns,
tableprinter.WithPadding(paddingFunc),
tableprinter.WithColor(cs.TableHeader),
tableprinter.WithColor(cs.GrayUnderline),
)
}

Expand All @@ -103,12 +98,3 @@ func WithHeader(columns ...string) 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: 0 additions & 47 deletions internal/tableprinter/table_printer_test.go

This file was deleted.

4 changes: 4 additions & 0 deletions internal/text/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,7 @@ func DisplayURL(urlStr string) string {
func RemoveDiacritics(value string) string {
return text.RemoveDiacritics(value)
}

func PadRight(maxWidth int, s string) string {
return text.PadRight(maxWidth, s)
}
1 change: 0 additions & 1 deletion pkg/cmd/gist/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ func listRun(opts *ListOptions) error {
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))
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/issue/shared/display.go
Original file line number Diff line number Diff line change
Expand Up @@ -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), tableprinter.WithTruncate(tableprinter.TruncateNonURL))
table.AddField(text.RemoveExcessiveWhitespace(issue.Title))
table.AddField(issueLabelList(&issue, cs, isTTY))
table.AddTimeField(now, issue.UpdatedAt, cs.Gray)
table.EndRow()
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/label/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func printLabels(io *iostreams.IOStreams, labels []label) error {

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

table.EndRow()
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/pr/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func listRun(opts *ListOptions) error {
}

table.AddField(prNum, tableprinter.WithColor(cs.ColorFromString(shared.ColorForPRState(pr))))
table.AddField(text.RemoveExcessiveWhitespace(pr.Title), tableprinter.WithTruncate(tableprinter.TruncateNonURL))
table.AddField(text.RemoveExcessiveWhitespace(pr.Title))
table.AddField(pr.HeadLabel(), tableprinter.WithColor(cs.Cyan))
if !isTTY {
table.AddField(prStateWithDraft(&pr))
Expand Down
4 changes: 4 additions & 0 deletions pkg/cmd/project/field-list/field_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ func TestRunList_User(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
Expand Down Expand Up @@ -256,6 +257,7 @@ func TestRunList_Org(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
Expand Down Expand Up @@ -339,6 +341,7 @@ func TestRunList_Me(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
Expand Down Expand Up @@ -406,6 +409,7 @@ func TestRunList_Empty(t *testing.T) {

ios, _, _, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/project/item-list/item_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ func TestRunList_User(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
Expand Down Expand Up @@ -285,6 +286,7 @@ func TestRunList_Org(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
Expand Down Expand Up @@ -382,6 +384,7 @@ func TestRunList_Me(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
number: 1,
Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/project/list/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ func TestRunList(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
owner: "monalisa",
Expand Down Expand Up @@ -225,6 +226,7 @@ func TestRunList_Me(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
owner: "@me",
Expand Down Expand Up @@ -296,6 +298,7 @@ func TestRunListViewer(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{},
client: client,
Expand Down Expand Up @@ -374,6 +377,7 @@ func TestRunListOrg(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
owner: "github",
Expand Down Expand Up @@ -428,6 +432,7 @@ func TestRunListEmpty(t *testing.T) {

ios, _, _, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{},
client: client,
Expand Down Expand Up @@ -504,6 +509,7 @@ func TestRunListWithClosed(t *testing.T) {

ios, _, stdout, _ := iostreams.Test()
config := listConfig{
//nolint:staticcheck // SA1019: tableprinter.NoHeaders temporarily allowed in project tests.
tp: tableprinter.New(ios, tableprinter.NoHeader),
opts: listOpts{
owner: "monalisa",
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/release/view/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ func renderReleaseTTY(io *iostreams.IOStreams, release *shared.Release) error {
//nolint:staticcheck // SA1019: Showing NAME|SIZE headers adds nothing to table.
table := tableprinter.New(io, tableprinter.NoHeader)
for _, a := range release.Assets {
table.AddField(a.Name, tableprinter.WithTruncate(tableprinter.TruncateNonURL))
table.AddField(humanFileSize(a.Size), tableprinter.WithTruncate(tableprinter.TruncateNonURL))
table.AddField(a.Name)
table.AddField(humanFileSize(a.Size))
table.EndRow()
}
err := table.Render()
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/repo/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func listRun(opts *ListOptions) error {
}

tp.AddField(repo.NameWithOwner, tableprinter.WithColor(cs.Bold))
tp.AddField(text.RemoveExcessiveWhitespace(repo.Description), tableprinter.WithTruncate(tableprinter.TruncateNonURL))
tp.AddField(text.RemoveExcessiveWhitespace(repo.Description))
tp.AddField(info, tableprinter.WithColor(infoColor))
if tp.IsTTY() {
tp.AddField(text.FuzzyAgoAbbr(opts.Now(), *t), tableprinter.WithColor(cs.Gray))
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/status/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,9 +688,9 @@ func statusRun(opts *StatusOptions) error {
}
tp.AddField(si.Identifier, tableprinter.WithColor(idStyle), tableprinter.WithTruncate(nil))
if si.Reason != "" {
tp.AddField(si.Reason, tableprinter.WithTruncate(tableprinter.TruncateNonURL))
tp.AddField(si.Reason)
}
tp.AddField(si.Preview(), tableprinter.WithTruncate(tableprinter.TruncateNonURL))
tp.AddField(si.Preview())
tp.EndRow()
}

Expand Down
37 changes: 18 additions & 19 deletions pkg/iostreams/color.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,21 @@ import (
)

var (
magenta = ansi.ColorFunc("magenta")
cyan = ansi.ColorFunc("cyan")
red = ansi.ColorFunc("red")
yellow = ansi.ColorFunc("yellow")
blue = ansi.ColorFunc("blue")
green = ansi.ColorFunc("green")
gray = ansi.ColorFunc("black+h")
bold = ansi.ColorFunc("default+b")
cyanBold = ansi.ColorFunc("cyan+b")
greenBold = ansi.ColorFunc("green+b")
magenta = ansi.ColorFunc("magenta")
cyan = ansi.ColorFunc("cyan")
red = ansi.ColorFunc("red")
yellow = ansi.ColorFunc("yellow")
blue = ansi.ColorFunc("blue")
green = ansi.ColorFunc("green")
gray = ansi.ColorFunc("black+h")
grayUnderline = ansi.ColorFunc("white+du")
bold = ansi.ColorFunc("default+b")
cyanBold = ansi.ColorFunc("cyan+b")
greenBold = ansi.ColorFunc("green+b")

gray256 = func(t string) string {
return fmt.Sprintf("\x1b[%d;5;%dm%s\x1b[m", 38, 242, t)
}

tableHeader = ansi.ColorFunc("white+du")
)

func NewColorScheme(enabled, is256enabled bool, trueColor bool) *ColorScheme {
Expand Down Expand Up @@ -110,6 +109,13 @@ func (c *ColorScheme) Grayf(t string, args ...interface{}) string {
return c.Gray(fmt.Sprintf(t, args...))
}

func (c *ColorScheme) GrayUnderline(t string) string {
if !c.enabled {
return t
}
return grayUnderline(t)
}

func (c *ColorScheme) Magenta(t string) string {
if !c.enabled {
return t
Expand Down Expand Up @@ -170,13 +176,6 @@ func (c *ColorScheme) FailureIconWithColor(colo func(string) string) string {
return colo("X")
}

func (c *ColorScheme) TableHeader(t string) string {
if !c.enabled {
return t
}
return tableHeader(t)
}

func (c *ColorScheme) ColorFromString(s string) func(string) string {
s = strings.ToLower(s)
var fn func(string) string
Expand Down

0 comments on commit d727f0b

Please sign in to comment.