Skip to content

Commit

Permalink
Fix column alignment and truncation for Eastern Asian languages
Browse files Browse the repository at this point in the history
- Ensure that text is never truncated mid-character, which would result
  in garbled text

- Ensure that columns in `issue/pr list` output align properly
  • Loading branch information
mislav committed Feb 20, 2020
1 parent cbc2d16 commit 4c3e498
Show file tree
Hide file tree
Showing 7 changed files with 148 additions and 24 deletions.
3 changes: 2 additions & 1 deletion command/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/cli/cli/git"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/githubtemplate"
"github.com/cli/cli/pkg/text"
"github.com/cli/cli/utils"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
Expand Down Expand Up @@ -407,7 +408,7 @@ func printIssues(w io.Writer, prefix string, totalCount int, issues []api.Issue)
ago := now.Sub(issue.UpdatedAt)

fmt.Fprintf(w, "%s%s %s%s %s\n", prefix, number,
truncate(70, replaceExcessiveWhitespace(issue.Title)),
text.Truncate(70, replaceExcessiveWhitespace(issue.Title)),
coloredLabels,
utils.Gray(utils.FuzzyAgo(ago)))
}
Expand Down
10 changes: 2 additions & 8 deletions command/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/cli/cli/context"
"github.com/cli/cli/git"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/text"
"github.com/cli/cli/utils"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
Expand Down Expand Up @@ -384,7 +385,7 @@ func prSelectorForCurrentBranch(ctx context.Context) (prNumber int, prHeadRef st
func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) {
for _, pr := range prs {
prNumber := fmt.Sprintf("#%d", pr.Number)
fmt.Fprintf(w, " %s %s %s", utils.Green(prNumber), truncate(50, replaceExcessiveWhitespace(pr.Title)), utils.Cyan("["+pr.HeadLabel()+"]"))
fmt.Fprintf(w, " %s %s %s", utils.Green(prNumber), text.Truncate(50, replaceExcessiveWhitespace(pr.Title)), utils.Cyan("["+pr.HeadLabel()+"]"))

checks := pr.ChecksStatus()
reviews := pr.ReviewStatus()
Expand Down Expand Up @@ -432,13 +433,6 @@ func printMessage(w io.Writer, s string) {
fmt.Fprintln(w, utils.Gray(s))
}

func truncate(maxLength int, title string) string {
if len(title) > maxLength {
return title[0:maxLength-3] + "..."
}
return title
}

func replaceExcessiveWhitespace(s string) string {
s = strings.TrimSpace(s)
s = regexp.MustCompile(`\r?\n`).ReplaceAllString(s, " ")
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ require (
golang.org/x/crypto v0.0.0-20200219234226-1ad67e1f0ef4
golang.org/x/net v0.0.0-20200219183655-46282727080f // indirect
golang.org/x/sys v0.0.0-20200219091948-cb0a6d8edb6c // indirect
golang.org/x/text v0.3.0
gopkg.in/yaml.v2 v2.2.8 // indirect
gopkg.in/yaml.v3 v3.0.0-20200121175148-a6ecf24a6d71
)
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ golang.org/x/sys v0.0.0-20190530182044-ad28b68e88f1/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200219091948-cb0a6d8edb6c h1:jceGD5YNJGgGMkJz79agzOln1K9TaZUjv5ird16qniQ=
golang.org/x/sys v0.0.0-20200219091948-cb0a6d8edb6c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
Expand Down
63 changes: 63 additions & 0 deletions pkg/text/truncate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package text

import (
"golang.org/x/text/width"
)

// DisplayWidth calculates what the rendered width of a string may be
func DisplayWidth(s string) int {
w := 0
for _, r := range s {
w += runeDisplayWidth(r)
}
return w
}

const (
ellipsisWidth = 3
minWidthForEllipsis = 5
)

// Truncate shortens a string to fit the maximum display width
func Truncate(max int, s string) string {
w := DisplayWidth(s)
if w <= max {
return s
}

useEllipsis := false
if max >= minWidthForEllipsis {
useEllipsis = true
max -= 3
}

cw := 0
ri := 0
for _, r := range s {
rw := runeDisplayWidth(r)
if cw+rw > max {
break
}
cw += rw
ri++
}

res := string([]rune(s)[:ri])
if useEllipsis {
res += "..."
}
if cw < max {
// compensate if truncating a wide character left an odd space
res += " "
}
return res
}

func runeDisplayWidth(r rune) int {
switch width.LookupRune(r).Kind() {
case width.EastAsianWide, width.EastAsianAmbiguous, width.EastAsianFullwidth:
return 2
default:
return 1
}
}
71 changes: 71 additions & 0 deletions pkg/text/truncate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package text

import "testing"

func TestTruncate(t *testing.T) {
type args struct {
max int
s string
}
tests := []struct {
name string
args args
want string
}{
{
name: "Japanese",
args: args{
max: 11,
s: "テストテストテストテスト",
},
want: "テストテ...",
},
{
name: "Japanese filled",
args: args{
max: 11,
s: "aテストテストテストテスト",
},
want: "aテスト... ",
},
{
name: "Chinese",
args: args{
max: 11,
s: "幫新舉報違章工廠新增編號",
},
want: "幫新舉報...",
},
{
name: "Chinese filled",
args: args{
max: 11,
s: "a幫新舉報違章工廠新增編號",
},
want: "a幫新舉... ",
},
{
name: "Korean",
args: args{
max: 11,
s: "프로젝트 내의",
},
want: "프로젝트...",
},
{
name: "Korean filled",
args: args{
max: 11,
s: "a프로젝트 내의",
},
want: "a프로젝... ",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := Truncate(tt.args.max, tt.args.s); got != tt.want {
t.Errorf("Truncate() = %q, want %q", got, tt.want)
}
})
}
}
23 changes: 8 additions & 15 deletions utils/table_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strconv"
"strings"

"github.com/cli/cli/pkg/text"
"github.com/mattn/go-isatty"
"golang.org/x/crypto/ssh/terminal"
)
Expand Down Expand Up @@ -62,16 +63,16 @@ func (t ttyTablePrinter) IsTTY() bool {
return true
}

func (t *ttyTablePrinter) AddField(text string, truncateFunc func(int, string) string, colorFunc func(string) string) {
func (t *ttyTablePrinter) AddField(s string, truncateFunc func(int, string) string, colorFunc func(string) string) {
if truncateFunc == nil {
truncateFunc = truncate
truncateFunc = text.Truncate
}
if t.rows == nil {
t.rows = [][]tableField{[]tableField{}}
}
rowI := len(t.rows) - 1
field := tableField{
Text: text,
Text: s,
TruncateFunc: truncateFunc,
ColorFunc: colorFunc,
}
Expand All @@ -92,7 +93,7 @@ func (t *ttyTablePrinter) Render() error {
// measure maximum content width per column
for _, row := range t.rows {
for col, field := range row {
textLen := len(field.Text)
textLen := text.DisplayWidth(field.Text)
if textLen > colWidths[col] {
colWidths[col] = textLen
}
Expand Down Expand Up @@ -128,7 +129,9 @@ func (t *ttyTablePrinter) Render() error {
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 padWidth := colWidths[col] - text.DisplayWidth(field.Text); padWidth > 0 {
truncVal += strings.Repeat(" ", padWidth)
}
}
if field.ColorFunc != nil {
truncVal = field.ColorFunc(truncVal)
Expand Down Expand Up @@ -173,13 +176,3 @@ func (t *tsvTablePrinter) EndRow() {
func (t *tsvTablePrinter) Render() error {
return nil
}

func truncate(maxLength int, title string) string {
if len(title) > maxLength {
if maxLength > 3 {
return title[0:maxLength-3] + "..."
}
return title[0:maxLength]
}
return title
}

0 comments on commit 4c3e498

Please sign in to comment.