fix(diffview): add fixes + more extensive testing for width handling

Andrey Nering created 5 months ago

Change summary

internal/exp/diffview/Taskfile.yaml    | 20 ++++++++++++++++++
internal/exp/diffview/diffview.go      | 26 +++++++++++++++--------
internal/exp/diffview/diffview_test.go | 31 ++++++++++++++++++++++++++++
internal/exp/diffview/util.go          | 15 +++++++++++++
4 files changed, 83 insertions(+), 9 deletions(-)

Detailed changes

internal/exp/diffview/Taskfile.yaml

@@ -20,3 +20,23 @@ tasks:
       - for: sources
         cmd: echo && echo "------- {{.ITEM}} -------" && echo && cat {{.ITEM}}
     silent: true
+
+  test:print:width:unified:
+    desc: Print golden files for debugging
+    method: none
+    sources:
+      - ./testdata/TestDiffViewWidth/Unified/LightMode/*.golden
+    cmds:
+      - for: sources
+        cmd: echo && echo "------- {{.ITEM}} -------" && echo && cat {{.ITEM}}
+    silent: true
+
+  test:print:width:split:
+    desc: Print golden files for debugging
+    method: none
+    sources:
+      - ./testdata/TestDiffViewWidth/Split/LightMode/*.golden
+    cmds:
+      - for: sources
+        cmd: echo && echo "------- {{.ITEM}} -------" && echo && cat {{.ITEM}}
+    silent: true

internal/exp/diffview/diffview.go

@@ -49,7 +49,8 @@ type DiffView struct {
 	splitHunks []splitHunk
 
 	codeWidth       int
-	fullCodeWidth   int // with leading symbols
+	fullCodeWidth   int  // with leading symbols
+	extraColOnAfter bool // add extra column on after panel
 	beforeNumDigits int
 	afterNumDigits  int
 }
@@ -144,11 +145,16 @@ func (dv *DiffView) String() string {
 		dv.resizeCodeWidth()
 	}
 
+	style := lipgloss.NewStyle()
+	if dv.width > 0 {
+		style = style.MaxWidth(dv.width)
+	}
+
 	switch dv.layout {
 	case layoutUnified:
-		return dv.renderUnified()
+		return style.Render(strings.TrimSuffix(dv.renderUnified(), "\n"))
 	case layoutSplit:
-		return dv.renderSplit()
+		return style.Render(strings.TrimSuffix(dv.renderSplit(), "\n"))
 	default:
 		panic("unknown diffview layout")
 	}
@@ -268,7 +274,9 @@ func (dv *DiffView) resizeCodeWidth() {
 	case layoutUnified:
 		dv.codeWidth = dv.width - fullNumWidth - leadingSymbolsSize
 	case layoutSplit:
-		dv.codeWidth = (dv.width - fullNumWidth - leadingSymbolsSize*2) / 2
+		remainingWidth := dv.width - fullNumWidth - leadingSymbolsSize*2
+		dv.codeWidth = remainingWidth / 2
+		dv.extraColOnAfter = isOdd(remainingWidth)
 	}
 
 	dv.fullCodeWidth = dv.codeWidth + leadingSymbolsSize
@@ -340,7 +348,7 @@ func (dv *DiffView) renderSplit() string {
 		if dv.lineNumbers {
 			b.WriteString(dv.style.DividerLine.LineNumber.Render(pad("…", dv.afterNumDigits)))
 		}
-		b.WriteString(dv.style.DividerLine.Code.Width(dv.fullCodeWidth).Render(" "))
+		b.WriteString(dv.style.DividerLine.Code.Width(dv.fullCodeWidth + btoi(dv.extraColOnAfter)).Render(" "))
 		b.WriteRune('\n')
 
 		beforeLine := h.fromLine
@@ -355,7 +363,7 @@ func (dv *DiffView) renderSplit() string {
 			}
 			if l.after != nil {
 				afterContent = strings.TrimSuffix(l.after.Content, "\n")
-				afterContent = ansi.Truncate(afterContent, dv.codeWidth, "…")
+				afterContent = ansi.Truncate(afterContent, dv.codeWidth+btoi(dv.extraColOnAfter), "…")
 			}
 
 			switch {
@@ -384,19 +392,19 @@ func (dv *DiffView) renderSplit() string {
 				if dv.lineNumbers {
 					b.WriteString(dv.style.MissingLine.LineNumber.Render(pad(" ", dv.afterNumDigits)))
 				}
-				b.WriteString(dv.style.MissingLine.Code.Width(dv.fullCodeWidth).Render("  "))
+				b.WriteString(dv.style.MissingLine.Code.Width(dv.fullCodeWidth + btoi(dv.extraColOnAfter)).Render("  "))
 			case l.after.Kind == udiff.Equal:
 				if dv.lineNumbers {
 					b.WriteString(dv.style.EqualLine.LineNumber.Render(pad(afterLine, dv.afterNumDigits)))
 				}
-				b.WriteString(dv.style.EqualLine.Code.Width(dv.fullCodeWidth).Render("  " + afterContent))
+				b.WriteString(dv.style.EqualLine.Code.Width(dv.fullCodeWidth + btoi(dv.extraColOnAfter)).Render("  " + afterContent))
 				afterLine++
 			case l.after.Kind == udiff.Insert:
 				if dv.lineNumbers {
 					b.WriteString(dv.style.InsertLine.LineNumber.Render(pad(afterLine, dv.afterNumDigits)))
 				}
 				b.WriteString(dv.style.InsertLine.Symbol.Render("+ "))
-				b.WriteString(dv.style.InsertLine.Code.Width(dv.codeWidth).Render(afterContent))
+				b.WriteString(dv.style.InsertLine.Code.Width(dv.codeWidth + btoi(dv.extraColOnAfter)).Render(afterContent))
 				afterLine++
 			}
 

internal/exp/diffview/diffview_test.go

@@ -2,6 +2,7 @@ package diffview_test
 
 import (
 	_ "embed"
+	"fmt"
 	"strings"
 	"testing"
 
@@ -136,6 +137,36 @@ func TestDiffView(t *testing.T) {
 	}
 }
 
+func TestDiffViewWidth(t *testing.T) {
+	for layoutName, layoutFunc := range LayoutFuncs {
+		t.Run(layoutName, func(t *testing.T) {
+			for themeName, themeFunc := range ThemeFuncs {
+				t.Run(themeName, func(t *testing.T) {
+					for width := 1; width <= 110; width++ {
+						if layoutName == "Unified" && width > 60 {
+							continue
+						}
+
+						t.Run(fmt.Sprintf("WidthOf%03d", width), func(t *testing.T) {
+							dv := diffview.New().
+								Before("main.go", TestMultipleHunksBefore).
+								After("main.go", TestMultipleHunksAfter).
+								Width(width)
+							dv = layoutFunc(dv)
+							dv = themeFunc(dv)
+
+							output := dv.String()
+							golden.RequireEqual(t, []byte(output))
+
+							assertLineWidth(t, width, output)
+						})
+					}
+				})
+			}
+		})
+	}
+}
+
 func assertLineWidth(t *testing.T, expected int, output string) {
 	var lineWidth int
 	for line := range strings.SplitSeq(output, "\n") {

internal/exp/diffview/util.go

@@ -15,3 +15,18 @@ func pad(v any, width int) string {
 	}
 	return strings.Repeat(" ", width-w) + s
 }
+
+func isEven(n int) bool {
+	return n%2 == 0
+}
+
+func isOdd(n int) bool {
+	return !isEven(n)
+}
+
+func btoi(b bool) int {
+	if b {
+		return 1
+	}
+	return 0
+}