Repository: cli/cli
Pull request #1677 (2020-09-11 09:39:05)

Fix issue listing misalignment
Improve logic for determining string truncation and string display width. This new logic uses a combination of https://github.com/mattn/go-runewidth and https://github.com/rivo/uniseg. This should result in better alignment when using the issue list command.

Note: `func graphemeWidth` is not always 100% accurate so there might still be some misalignment cases that exist but it has more robust rune recognition (due to go-runewidth) than our previous implementation. 

closes https://github.com/cli/cli/issues/1454


Patch:
diff --git a/cmd/gh/main.go b/cmd/gh/main.go
index e61bb1c6c..44357ffd6 100644
--- a/cmd/gh/main.go
+++ b/cmd/gh/main.go
@@ -94,7 +94,7 @@ func main() {
 
 	authCheckEnabled := os.Getenv("GITHUB_TOKEN") == "" &&
 		os.Getenv("GITHUB_ENTERPRISE_TOKEN") == "" &&
-		cmd != nil && cmdutil.IsAuthCheckEnabled(cmd)
+		cmdutil.IsAuthCheckEnabled(cmd)
 	if authCheckEnabled {
 		cfg, err := cmdFactory.Config()
 		if err != nil {
diff --git a/docs/install_linux.md b/docs/install_linux.md
index 4a21c0094..30052d808 100644
--- a/docs/install_linux.md
+++ b/docs/install_linux.md
@@ -43,20 +43,8 @@ sudo dnf install gh
 
 ### openSUSE/SUSE Linux (zypper)
 
-Install:
-
-```bash
-sudo zypper addrepo https://cli.github.com/packages/rpm/gh-cli.repo
-sudo zypper ref
-sudo zypper install gh
-```
-
-Upgrade:
-
-```bash
-sudo zypper ref
-sudo zypper update gh
-```
+It's possible that https://cli.github.com/packages/rpm/gh-cli.repo will work with zypper, but
+this hasn't been tested.
 
 ## Manual installation
 
diff --git a/go.mod b/go.mod
index 42a005291..47afd50ae 100644
--- a/go.mod
+++ b/go.mod
@@ -14,15 +14,17 @@ require (
 	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
 	github.com/mattn/go-colorable v0.1.7
 	github.com/mattn/go-isatty v0.0.12
+	github.com/mattn/go-runewidth v0.0.9
 	github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
 	github.com/mitchellh/go-homedir v1.1.0
+	github.com/rivo/uniseg v0.1.0
 	github.com/shurcooL/githubv4 v0.0.0-20200802174311-f27d2ca7f6d5
 	github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f
 	github.com/spf13/cobra v1.0.0
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.6.1
 	golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a
-	golang.org/x/text v0.3.3
+	golang.org/x/text v0.3.3 // indirect
 	gopkg.in/yaml.v2 v2.2.8 // indirect
 	gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776
 )
diff --git a/go.sum b/go.sum
index 107d9a245..f9f02eabe 100644
--- a/go.sum
+++ b/go.sum
@@ -151,6 +151,8 @@ github.com/prometheus/common v0.4.0/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y8
 github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
 github.com/prometheus/procfs v0.0.0-20190507164030-5867b95ac084/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA=
 github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
+github.com/rivo/uniseg v0.1.0 h1:+2KBaVoUmb9XzDsrx/Ct0W/EYOSFf/nWTauy++DprtY=
+github.com/rivo/uniseg v0.1.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
 github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
 github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0RK8m9o+Q=
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go
index 7896d1d71..adcf384a4 100644
--- a/pkg/cmd/repo/view/view.go
+++ b/pkg/cmd/repo/view/view.go
@@ -2,9 +2,9 @@ package view
 
 import (
 	"fmt"
+	"html/template"
 	"net/http"
 	"strings"
-	"text/template"
 
 	"github.com/MakeNowJust/heredoc"
 	"github.com/cli/cli/api"
diff --git a/pkg/text/truncate.go b/pkg/text/truncate.go
index 78cd05da2..98a924c3e 100644
--- a/pkg/text/truncate.go
+++ b/pkg/text/truncate.go
@@ -1,80 +1,75 @@
 package text
 
 import (
-	"golang.org/x/text/width"
+	runewidth "github.com/mattn/go-runewidth"
+	"github.com/rivo/uniseg"
+)
+
+const (
+	ellipsisWidth       = 3
+	minWidthForEllipsis = 5
 )
 
 // DisplayWidth calculates what the rendered width of a string may be
 func DisplayWidth(s string) int {
+	g := uniseg.NewGraphemes(s)
 	w := 0
-	for _, r := range s {
-		w += runeDisplayWidth(r)
+	for g.Next() {
+		w += graphemeWidth(g)
 	}
 	return w
 }
 
-const (
-	ellipsisWidth       = 3
-	minWidthForEllipsis = 5
-)
-
 // Truncate shortens a string to fit the maximum display width
-func Truncate(max int, s string) string {
+func Truncate(maxWidth int, s string) string {
 	w := DisplayWidth(s)
-	if w <= max {
+	if w <= maxWidth {
 		return s
 	}
 
 	useEllipsis := false
-	if max >= minWidthForEllipsis {
+	if maxWidth >= minWidthForEllipsis {
 		useEllipsis = true
-		max -= ellipsisWidth
+		maxWidth -= ellipsisWidth
 	}
 
-	cw := 0
-	ri := 0
-	for _, r := range s {
-		rw := runeDisplayWidth(r)
-		if cw+rw > max {
+	g := uniseg.NewGraphemes(s)
+	r := ""
+	rWidth := 0
+	for {
+		g.Next()
+		gWidth := graphemeWidth(g)
+
+		if rWidth+gWidth <= maxWidth {
+			r += g.Str()
+			rWidth += gWidth
+			continue
+		} else {
 			break
 		}
-		cw += rw
-		ri++
 	}
 
-	res := string([]rune(s)[:ri])
 	if useEllipsis {
-		res += "..."
+		r += "..."
 	}
-	if cw < max {
-		// compensate if truncating a wide character left an odd space
-		res += " "
+
+	if rWidth < maxWidth {
+		r += " "
 	}
-	return res
-}
 
-var runeDisplayWidthOverrides = map[rune]int{
-	'“': 1,
-	'”': 1,
-	'‘': 1,
-	'’': 1,
-	'–': 1, // en dash
-	'—': 1, // em dash
-	'→': 1,
-	'…': 1,
-	'•': 1, // bullet
-	'·': 1, // middle dot
+	return r
 }
 
-func runeDisplayWidth(r rune) int {
-	if w, ok := runeDisplayWidthOverrides[r]; ok {
-		return w
-	}
-
-	switch width.LookupRune(r).Kind() {
-	case width.EastAsianWide, width.EastAsianAmbiguous, width.EastAsianFullwidth:
-		return 2
-	default:
-		return 1
+// graphemeWidth calculates what the rendered width of a grapheme may be
+func graphemeWidth(g *uniseg.Graphemes) int {
+	// If grapheme spans one rune use that rune width
+	// If grapheme spans multiple runes use the first non-zero rune width
+	w := 0
+	for _, r := range g.Runes() {
+		w = runewidth.RuneWidth(r)
+		if w > 0 {
+			break
+		}
 	}
+	return w
 }


Test patch:
diff --git a/pkg/cmd/repo/view/view_test.go b/pkg/cmd/repo/view/view_test.go
index 626ff62ed..7dba0f6df 100644
--- a/pkg/cmd/repo/view/view_test.go
+++ b/pkg/cmd/repo/view/view_test.go
@@ -515,86 +515,3 @@ func Test_ViewRun_WithoutUsername(t *testing.T) {
 	assert.Equal(t, "", stderr.String())
 	reg.Verify(t)
 }
-
-func Test_ViewRun_HandlesSpecialCharacters(t *testing.T) {
-	tests := []struct {
-		name       string
-		opts       *ViewOptions
-		repoName   string
-		stdoutTTY  bool
-		wantOut    string
-		wantStderr string
-		wantErr    bool
-	}{
-		{
-			name: "nontty",
-			wantOut: heredoc.Doc(`
-				name:	OWNER/REPO
-				description:	Some basic special characters " & / < > '
-				--
-				# < is always > than & ' and "
-				`),
-		},
-		{
-			name:      "no args",
-			stdoutTTY: true,
-			wantOut: heredoc.Doc(`
-				OWNER/REPO
-				Some basic special characters " & / < > '
-
-
-				  # < is always > than & ' and "                                              
-
-
-
-				View this repository on GitHub: https://github.com/OWNER/REPO
-			`),
-		},
-	}
-	for _, tt := range tests {
-		if tt.opts == nil {
-			tt.opts = &ViewOptions{}
-		}
-
-		if tt.repoName == "" {
-			tt.repoName = "OWNER/REPO"
-		}
-
-		tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
-			repo, _ := ghrepo.FromFullName(tt.repoName)
-			return repo, nil
-		}
-
-		reg := &httpmock.Registry{}
-		reg.Register(
-			httpmock.GraphQL(`query RepositoryInfo\b`),
-			httpmock.StringResponse(`
-		{ "data": {
-			"repository": {
-			"description": "Some basic special characters \" & / < > '"
-		} } }`))
-		reg.Register(
-			httpmock.REST("GET", fmt.Sprintf("repos/%s/readme", tt.repoName)),
-			httpmock.StringResponse(`
-		{ "name": "readme.md",
-		"content": "IyA8IGlzIGFsd2F5cyA+IHRoYW4gJiAnIGFuZCAi"}`))
-
-		tt.opts.HttpClient = func() (*http.Client, error) {
-			return &http.Client{Transport: reg}, nil
-		}
-
-		io, _, stdout, stderr := iostreams.Test()
-		tt.opts.IO = io
-
-		t.Run(tt.name, func(t *testing.T) {
-			io.SetStdoutTTY(tt.stdoutTTY)
-
-			if err := viewRun(tt.opts); (err != nil) != tt.wantErr {
-				t.Errorf("viewRun() error = %v, wantErr %v", err, tt.wantErr)
-			}
-			assert.Equal(t, tt.wantStderr, stderr.String())
-			assert.Equal(t, tt.wantOut, stdout.String())
-			reg.Verify(t)
-		})
-	}
-}
diff --git a/pkg/text/truncate_test.go b/pkg/text/truncate_test.go
index 85a399e80..95119e2ff 100644
--- a/pkg/text/truncate_test.go
+++ b/pkg/text/truncate_test.go
@@ -62,6 +62,22 @@ func TestTruncate(t *testing.T) {
 			},
 			want: "a프로젝... ",
 		},
+		{
+			name: "Emoji",
+			args: args{
+				max: 11,
+				s:   "💡💡💡💡💡💡💡💡💡💡💡💡",
+			},
+			want: "💡💡💡💡...",
+		},
+		{
+			name: "Accented characters",
+			args: args{
+				max: 11,
+				s:   "é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́é́́",
+			},
+			want: "é́́é́́é́́é́́é́́é́́é́́é́́...",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -128,6 +144,11 @@ func TestDisplayWidth(t *testing.T) {
 			text: `👍`,
 			want: 2,
 		},
+		{
+			name: "accent character",
+			text: `é́́`,
+			want: 1,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {