From f30e973b9db937b6888b3d51408935e2648cfccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 15 Nov 2019 19:19:41 +0100 Subject: [PATCH 1/4] Extract generic row printer that adjusts itself for receiving terminal This makes the approach from `pr list` reusable across other commands that may benefit from table-based output, e.g. `issue list` or `pr status` The idea is: instantiate a printer, connect it to stdout, feed it some data, and it does the rest: colored, truncated column output that fits into a terminal, or tab-delimited output (no color, no truncation) for scripts. --- command/pr.go | 61 +++++++---------------- utils/table_printer.go | 108 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 43 deletions(-) create mode 100644 utils/table_printer.go diff --git a/command/pr.go b/command/pr.go index 0445b81d6..b7cca14a1 100644 --- a/command/pr.go +++ b/command/pr.go @@ -2,13 +2,11 @@ package command import ( "fmt" - "os" "strconv" "github.com/github/gh-cli/api" "github.com/github/gh-cli/utils" "github.com/spf13/cobra" - "golang.org/x/crypto/ssh/terminal" ) func init() { @@ -158,53 +156,30 @@ func prList(cmd *cobra.Command, args []string) error { return err } - tty := false - ttyWidth := 80 - out := cmd.OutOrStdout() - if outFile, isFile := out.(*os.File); isFile { - fd := int(outFile.Fd()) - tty = terminal.IsTerminal(fd) - if w, _, err := terminal.GetSize(fd); err == nil { - ttyWidth = w - } - } - - numWidth := 0 - maxTitleWidth := 0 + table := utils.NewTablePrinter(cmd.OutOrStdout()) for _, pr := range prs { - numLen := len(strconv.Itoa(pr.Number)) + 1 - if numLen > numWidth { - numWidth = numLen - } - if len(pr.Title) > maxTitleWidth { - maxTitleWidth = len(pr.Title) - } + table.SetContentWidth(0, len(strconv.Itoa(pr.Number))+1) + table.SetContentWidth(1, len(pr.Title)) + table.SetContentWidth(2, len(pr.HeadLabel())) } - branchWidth := 40 - titleWidth := ttyWidth - branchWidth - 2 - numWidth - 2 - - if maxTitleWidth < titleWidth { - branchWidth += titleWidth - maxTitleWidth - titleWidth = maxTitleWidth - } + table.FitColumns() + table.SetColorFunc(2, utils.Cyan) for _, pr := range prs { - if tty { - prNum := fmt.Sprintf("% *s", numWidth, fmt.Sprintf("#%d", pr.Number)) - switch pr.State { - case "OPEN": - prNum = utils.Green(prNum) - case "CLOSED": - prNum = utils.Red(prNum) - case "MERGED": - prNum = utils.Magenta(prNum) - } - prBranch := utils.Cyan(truncate(branchWidth, pr.HeadLabel())) - fmt.Fprintf(out, "%s %-*s %s\n", prNum, titleWidth, truncate(titleWidth, pr.Title), prBranch) - } else { - fmt.Fprintf(out, "%d\t%s\t%s\n", pr.Number, pr.Title, pr.HeadLabel()) + prNum := strconv.Itoa(pr.Number) + if table.IsTTY { + prNum = "#" + prNum } + switch pr.State { + case "OPEN": + table.SetColorFunc(0, utils.Green) + case "CLOSED": + table.SetColorFunc(0, utils.Red) + case "MERGED": + table.SetColorFunc(0, utils.Magenta) + } + table.WriteRow(prNum, pr.Title, pr.HeadLabel()) } return nil } diff --git a/utils/table_printer.go b/utils/table_printer.go new file mode 100644 index 000000000..fd38a928e --- /dev/null +++ b/utils/table_printer.go @@ -0,0 +1,108 @@ +package utils + +import ( + "fmt" + "io" + "os" + + "golang.org/x/crypto/ssh/terminal" +) + +func NewTablePrinter(w io.Writer) *TTYTablePrinter { + tty := false + ttyWidth := 80 + if outFile, isFile := w.(*os.File); isFile { + fd := int(outFile.Fd()) + tty = terminal.IsTerminal(fd) + if w, _, err := terminal.GetSize(fd); err == nil { + ttyWidth = w + } + } + return &TTYTablePrinter{ + out: w, + IsTTY: tty, + maxWidth: ttyWidth, + colWidths: []int{}, + colFuncs: make(map[int]func(string) string), + } +} + +type TTYTablePrinter struct { + out io.Writer + IsTTY bool + maxWidth int + colWidths []int + colFuncs map[int]func(string) string +} + +func (t *TTYTablePrinter) SetContentWidth(col, width int) { + if col == len(t.colWidths) { + t.colWidths = append(t.colWidths, 0) + } + if width > t.colWidths[col] { + t.colWidths[col] = width + } +} + +func (t *TTYTablePrinter) SetColorFunc(col int, colorize func(string) string) { + t.colFuncs[col] = colorize +} + +// FitColumns caps all but first column to fit available terminal width. +func (t *TTYTablePrinter) FitColumns() { + numCols := len(t.colWidths) + delimWidth := 2 + availWidth := t.maxWidth - t.colWidths[0] - ((numCols - 1) * delimWidth) + // TODO: avoid widening columns that already fit + // TODO: support weighted instead of even redistribution + for col := 1; col < len(t.colWidths); col++ { + t.colWidths[col] = availWidth / (numCols - 1) + } +} + +func (t *TTYTablePrinter) WriteRow(fields ...string) error { + lastCol := len(fields) - 1 + delim := "\t" + if t.IsTTY { + delim = " " + } + + for col, val := range fields { + if col > 0 { + _, err := fmt.Fprint(t.out, delim) + if err != nil { + return err + } + } + if t.IsTTY { + truncVal := truncate(t.colWidths[col], val) + if col != lastCol { + truncVal = fmt.Sprintf("%-*s", t.colWidths[col], truncVal) + } + if t.colFuncs[col] != nil { + truncVal = t.colFuncs[col](truncVal) + } + _, err := fmt.Fprint(t.out, truncVal) + if err != nil { + return err + } + } else { + _, err := fmt.Fprint(t.out, val) + if err != nil { + return err + } + } + } + _, err := fmt.Fprint(t.out, "\n") + if err != nil { + return err + } + return nil +} + +func truncate(maxLength int, title string) string { + if len(title) > maxLength { + return title[0:maxLength-3] + "..." + } + return title +} From 9fc80a1f8abcdb43366945f597d0b191738efb2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 20 Nov 2019 12:18:50 +0100 Subject: [PATCH 2/4] Fix crash with empty table --- utils/table_printer.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/utils/table_printer.go b/utils/table_printer.go index fd38a928e..fa66b5ee4 100644 --- a/utils/table_printer.go +++ b/utils/table_printer.go @@ -22,7 +22,7 @@ func NewTablePrinter(w io.Writer) *TTYTablePrinter { out: w, IsTTY: tty, maxWidth: ttyWidth, - colWidths: []int{}, + colWidths: make(map[int]int), colFuncs: make(map[int]func(string) string), } } @@ -31,14 +31,11 @@ type TTYTablePrinter struct { out io.Writer IsTTY bool maxWidth int - colWidths []int + colWidths map[int]int colFuncs map[int]func(string) string } func (t *TTYTablePrinter) SetContentWidth(col, width int) { - if col == len(t.colWidths) { - t.colWidths = append(t.colWidths, 0) - } if width > t.colWidths[col] { t.colWidths[col] = width } From 2022f8e74b83b09b3440ba395fa979535c92afbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 20 Nov 2019 12:30:24 +0100 Subject: [PATCH 3/4] Avoid widening table columns that already fit --- utils/table_printer.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/utils/table_printer.go b/utils/table_printer.go index fa66b5ee4..858c736ff 100644 --- a/utils/table_printer.go +++ b/utils/table_printer.go @@ -50,10 +50,19 @@ func (t *TTYTablePrinter) FitColumns() { numCols := len(t.colWidths) delimWidth := 2 availWidth := t.maxWidth - t.colWidths[0] - ((numCols - 1) * delimWidth) - // TODO: avoid widening columns that already fit + // add extra space from columns that are already narrower than threshold + for col := 1; col < len(t.colWidths); col++ { + availColWidth := availWidth / (numCols - 1) + if extra := availColWidth - t.colWidths[col]; extra > 0 { + availWidth += extra + } + } // TODO: support weighted instead of even redistribution for col := 1; col < len(t.colWidths); col++ { - t.colWidths[col] = availWidth / (numCols - 1) + availColWidth := availWidth / (numCols - 1) + if t.colWidths[col] > availColWidth { + t.colWidths[col] = availColWidth + } } } From 97a6dc494baefdca91011062ed6c1c5a41a903e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 20 Nov 2019 13:29:27 +0100 Subject: [PATCH 4/4] Redesign TablePrinter to avoid SetContentWidth / FitColumns steps The API is now: - AddField; - EndRow; - Render. --- command/pr.go | 42 +++++----- utils/table_printer.go | 172 +++++++++++++++++++++++++++-------------- 2 files changed, 137 insertions(+), 77 deletions(-) diff --git a/command/pr.go b/command/pr.go index 7135bbc55..7660e7740 100644 --- a/command/pr.go +++ b/command/pr.go @@ -167,33 +167,37 @@ func prList(cmd *cobra.Command, args []string) error { } table := utils.NewTablePrinter(cmd.OutOrStdout()) - for _, pr := range prs { - table.SetContentWidth(0, len(strconv.Itoa(pr.Number))+1) - table.SetContentWidth(1, len(pr.Title)) - table.SetContentWidth(2, len(pr.HeadLabel())) - } - - table.FitColumns() - table.SetColorFunc(2, utils.Cyan) - for _, pr := range prs { prNum := strconv.Itoa(pr.Number) - if table.IsTTY { + if table.IsTTY() { prNum = "#" + prNum } - switch pr.State { - case "OPEN": - table.SetColorFunc(0, utils.Green) - case "CLOSED": - table.SetColorFunc(0, utils.Red) - case "MERGED": - table.SetColorFunc(0, utils.Magenta) - } - table.WriteRow(prNum, pr.Title, pr.HeadLabel()) + table.AddField(prNum, nil, colorFuncForState(pr.State)) + table.AddField(pr.Title, nil, nil) + table.AddField(pr.HeadLabel(), nil, utils.Cyan) + table.EndRow() } + err = table.Render() + if err != nil { + return err + } + return nil } +func colorFuncForState(state string) func(string) string { + switch state { + case "OPEN": + return utils.Green + case "CLOSED": + return utils.Red + case "MERGED": + return utils.Magenta + default: + return nil + } +} + func prView(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) baseRepo, err := ctx.BaseRepo() diff --git a/utils/table_printer.go b/utils/table_printer.go index 858c736ff..1d763b8ce 100644 --- a/utils/table_printer.go +++ b/utils/table_printer.go @@ -8,101 +8,157 @@ import ( "golang.org/x/crypto/ssh/terminal" ) -func NewTablePrinter(w io.Writer) *TTYTablePrinter { - tty := false - ttyWidth := 80 +type TablePrinter interface { + IsTTY() bool + AddField(string, func(int, string) string, func(string) string) + EndRow() + Render() error +} + +func NewTablePrinter(w io.Writer) TablePrinter { if outFile, isFile := w.(*os.File); isFile { fd := int(outFile.Fd()) - tty = terminal.IsTerminal(fd) - if w, _, err := terminal.GetSize(fd); err == nil { - ttyWidth = w + if terminal.IsTerminal(fd) { + ttyWidth := 80 + if w, _, err := terminal.GetSize(fd); err == nil { + ttyWidth = w + } + return &ttyTablePrinter{ + out: w, + maxWidth: ttyWidth, + } } } - return &TTYTablePrinter{ - out: w, - IsTTY: tty, - maxWidth: ttyWidth, - colWidths: make(map[int]int), - colFuncs: make(map[int]func(string) string), + return &tsvTablePrinter{ + out: w, } } -type TTYTablePrinter struct { - out io.Writer - IsTTY bool - maxWidth int - colWidths map[int]int - colFuncs map[int]func(string) string +type tableField struct { + Text string + TruncateFunc func(int, string) string + ColorFunc func(string) string } -func (t *TTYTablePrinter) SetContentWidth(col, width int) { - if width > t.colWidths[col] { - t.colWidths[col] = width +type ttyTablePrinter struct { + out io.Writer + maxWidth int + rows [][]tableField +} + +func (t ttyTablePrinter) IsTTY() bool { + return true +} + +func (t *ttyTablePrinter) AddField(text string, truncateFunc func(int, string) string, colorFunc func(string) string) { + if truncateFunc == nil { + truncateFunc = truncate } + if t.rows == nil { + t.rows = [][]tableField{[]tableField{}} + } + rowI := len(t.rows) - 1 + field := tableField{ + Text: text, + TruncateFunc: truncateFunc, + ColorFunc: colorFunc, + } + t.rows[rowI] = append(t.rows[rowI], field) } -func (t *TTYTablePrinter) SetColorFunc(col int, colorize func(string) string) { - t.colFuncs[col] = colorize +func (t *ttyTablePrinter) EndRow() { + t.rows = append(t.rows, []tableField{}) } -// FitColumns caps all but first column to fit available terminal width. -func (t *TTYTablePrinter) FitColumns() { - numCols := len(t.colWidths) - delimWidth := 2 - availWidth := t.maxWidth - t.colWidths[0] - ((numCols - 1) * delimWidth) +func (t *ttyTablePrinter) Render() error { + if len(t.rows) == 0 { + return nil + } + + numCols := len(t.rows[0]) + colWidths := make([]int, numCols) + // measure maximum content width per column + for _, row := range t.rows { + for col, field := range row { + textLen := len(field.Text) + if textLen > colWidths[col] { + colWidths[col] = textLen + } + } + } + + delim := " " + availWidth := t.maxWidth - colWidths[0] - ((numCols - 1) * len(delim)) // add extra space from columns that are already narrower than threshold - for col := 1; col < len(t.colWidths); col++ { + for col := 1; col < numCols; col++ { availColWidth := availWidth / (numCols - 1) - if extra := availColWidth - t.colWidths[col]; extra > 0 { + if extra := availColWidth - colWidths[col]; extra > 0 { availWidth += extra } } + // cap all but first column to fit available terminal width // TODO: support weighted instead of even redistribution - for col := 1; col < len(t.colWidths); col++ { + for col := 1; col < numCols; col++ { availColWidth := availWidth / (numCols - 1) - if t.colWidths[col] > availColWidth { - t.colWidths[col] = availColWidth + if colWidths[col] > availColWidth { + colWidths[col] = availColWidth } } -} -func (t *TTYTablePrinter) WriteRow(fields ...string) error { - lastCol := len(fields) - 1 - delim := "\t" - if t.IsTTY { - delim = " " - } - - for col, val := range fields { - if col > 0 { - _, err := fmt.Fprint(t.out, delim) - if err != nil { - return err + for _, row := range t.rows { + for col, field := range row { + if col > 0 { + _, err := fmt.Fprint(t.out, delim) + if err != nil { + return err + } } - } - if t.IsTTY { - truncVal := truncate(t.colWidths[col], val) - if col != lastCol { - truncVal = fmt.Sprintf("%-*s", t.colWidths[col], truncVal) + truncVal := field.TruncateFunc(colWidths[col], field.Text) + if col < numCols-1 { + // pad value with spaces on the right + truncVal = fmt.Sprintf("%-*s", colWidths[col], truncVal) } - if t.colFuncs[col] != nil { - truncVal = t.colFuncs[col](truncVal) + if field.ColorFunc != nil { + truncVal = field.ColorFunc(truncVal) } _, err := fmt.Fprint(t.out, truncVal) if err != nil { return err } - } else { - _, err := fmt.Fprint(t.out, val) + } + if len(row) > 0 { + _, err := fmt.Fprint(t.out, "\n") if err != nil { return err } } } - _, err := fmt.Fprint(t.out, "\n") - if err != nil { - return err + return nil +} + +type tsvTablePrinter struct { + out io.Writer + currentCol int +} + +func (t tsvTablePrinter) IsTTY() bool { + return false +} + +func (t *tsvTablePrinter) AddField(text string, _ func(int, string) string, _ func(string) string) { + if t.currentCol > 0 { + fmt.Fprint(t.out, "\t") } + fmt.Fprint(t.out, text) + t.currentCol++ +} + +func (t *tsvTablePrinter) EndRow() { + fmt.Fprint(t.out, "\n") + t.currentCol = 0 +} + +func (t *tsvTablePrinter) Render() error { return nil }