fix: address code review findings

Author: Astro-Han

internal/git/diff.go

@@ -116,12 +116,21 @@ func getUntrackedDiff(repoDir string) ([]internal.FileDiff, error) {
 
 // buildNewFileDiff synthesizes an added-file diff for untracked content.
 func buildNewFileDiff(path, content string) internal.FileDiff {
+	if strings.ContainsRune(content, '\x00') {
+		return internal.FileDiff{
+			Path:     path,
+			Status:   internal.StatusBinary,
+			IsBinary: true,
+		}
+	}
+
 	lines := strings.Split(content, "\n")
+	if len(lines) > 0 && lines[len(lines)-1] == "" {
+		lines = lines[:len(lines)-1]
+	}
+
 	var diffLines []internal.DiffLine
 	for _, line := range lines {
-		if line == "" && len(diffLines) > 0 {
-			continue
-		}
 		diffLines = append(diffLines, internal.DiffLine{
 			Type:    internal.LineAdd,
 			Content: line,

internal/git/diff_test.go

@@ -4,6 +4,8 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+
+	"github.com/Astro-Han/diffpane/internal"
 )
 
 // TestComputeDiffModifiedFile verifies tracked modifications become unified diffs.
@@ -128,6 +130,68 @@ func TestComputeDiffFreshRepoUntrackedFile(t *testing.T) {
 	}
 }
 
+// TestComputeDiffUntrackedFilePreservesInnerBlankLines verifies synthetic diffs
+// keep blank lines inside file content instead of dropping them.
+func TestComputeDiffUntrackedFilePreservesInnerBlankLines(t *testing.T) {
+	root := initGitRepo(t)
+	runGit(t, root, "commit", "--allow-empty", "-m", "init")
+
+	baseline, err := GetHeadSHA(root)
+	if err != nil {
+		t.Fatalf("GetHeadSHA returned error: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(root, "blank.txt"), []byte("top\n\nbottom\n"), 0o600); err != nil {
+		t.Fatalf("write file: %v", err)
+	}
+
+	files, err := ComputeDiff(root, baseline)
+	if err != nil {
+		t.Fatalf("ComputeDiff returned error: %v", err)
+	}
+	if len(files) != 1 {
+		t.Fatalf("expected 1 file, got %d", len(files))
+	}
+	lines := files[0].Hunks[0].Lines
+	if len(lines) != 3 {
+		t.Fatalf("line count = %d, want 3", len(lines))
+	}
+	if lines[1].Content != "" {
+		t.Fatalf("middle line = %q, want blank line", lines[1].Content)
+	}
+	if files[0].AddCount != 3 {
+		t.Fatalf("add count = %d, want 3", files[0].AddCount)
+	}
+}
+
+// TestComputeDiffUntrackedBinaryFile verifies untracked binary files are not
+// rendered as text hunks.
+func TestComputeDiffUntrackedBinaryFile(t *testing.T) {
+	root := initGitRepo(t)
+	runGit(t, root, "commit", "--allow-empty", "-m", "init")
+
+	baseline, err := GetHeadSHA(root)
+	if err != nil {
+		t.Fatalf("GetHeadSHA returned error: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(root, "image.bin"), []byte{0x00, 0x01, 0x02}, 0o600); err != nil {
+		t.Fatalf("write file: %v", err)
+	}
+
+	files, err := ComputeDiff(root, baseline)
+	if err != nil {
+		t.Fatalf("ComputeDiff returned error: %v", err)
+	}
+	if len(files) != 1 {
+		t.Fatalf("expected 1 file, got %d", len(files))
+	}
+	if !files[0].IsBinary || files[0].Status != internal.StatusBinary {
+		t.Fatalf("expected binary status, got %#v", files[0])
+	}
+	if len(files[0].Hunks) != 0 {
+		t.Fatalf("binary file should not have text hunks, got %#v", files[0].Hunks)
+	}
+}
+
 // TestComputeDiffNoChanges verifies a clean worktree produces no diffs.
 func TestComputeDiffNoChanges(t *testing.T) {
 	root := initGitRepo(t)

internal/git/repo.go

@@ -91,6 +91,29 @@ func ResolveGitDir(repoDir string) string {
 	return filepath.Clean(filepath.Join(repoDir, gitDir))
 }
 
+// GetGitCommonDir returns the common git directory shared by linked worktrees.
+func GetGitCommonDir(repoDir string) string {
+	root, err := FindWorktreeRoot(repoDir)
+	if err != nil {
+		return ""
+	}
+
+	out, err := gitOutput(root, "rev-parse", "--git-common-dir")
+	if err != nil {
+		return ""
+	}
+
+	commonDir := strings.TrimSpace(out)
+	if commonDir == "" {
+		return ""
+	}
+	if filepath.IsAbs(commonDir) {
+		return commonDir
+	}
+
+	return filepath.Clean(filepath.Join(root, commonDir))
+}
+
 // gitOutput runs git in dir and returns stdout.
 func gitOutput(dir string, args ...string) (string, error) {
 	// #nosec G204 -- git is fixed and arguments are constructed by trusted callers.

internal/git/repo_test.go

@@ -160,3 +160,29 @@ func TestResolveGitDirLinkedWorktree(t *testing.T) {
 	want := filepath.Join(repo, ".git", "worktrees", "linked")
 	samePath(t, got, want)
 }
+
+// TestGetGitCommonDirNormalRepo resolves the common git dir for a standard repo.
+func TestGetGitCommonDirNormalRepo(t *testing.T) {
+	root := initGitRepo(t)
+
+	got := GetGitCommonDir(root)
+	want := filepath.Join(root, ".git")
+	samePath(t, got, want)
+}
+
+// TestGetGitCommonDirLinkedWorktree resolves the shared git dir for a linked worktree.
+func TestGetGitCommonDirLinkedWorktree(t *testing.T) {
+	repo := initGitRepo(t)
+	runGit(t, repo, "commit", "--allow-empty", "-m", "root")
+
+	worktrees := filepath.Join(t.TempDir(), "worktrees")
+	if err := os.MkdirAll(worktrees, 0o750); err != nil {
+		t.Fatalf("mkdir worktrees: %v", err)
+	}
+	worktreeDir := filepath.Join(worktrees, "linked")
+	runGit(t, repo, "worktree", "add", worktreeDir)
+
+	got := GetGitCommonDir(worktreeDir)
+	want := filepath.Join(repo, ".git")
+	samePath(t, got, want)
+}

internal/ui/diffview.go

@@ -22,11 +22,15 @@ func RenderDiffView(file *internal.FileDiff, scrollOffset, width, height int) st
 		for _, diffLine := range hunk.Lines {
 			switch diffLine.Type {
 			case internal.LineAdd:
-				lines = append(lines, StyleAdd.Render(wrapLine("+"+diffLine.Content, width)))
+				for _, visualLine := range wrapLineParts("+"+diffLine.Content, width) {
+					lines = append(lines, StyleAdd.Render(visualLine))
+				}
 			case internal.LineDel:
-				lines = append(lines, StyleDel.Render(wrapLine("-"+diffLine.Content, width)))
+				for _, visualLine := range wrapLineParts("-"+diffLine.Content, width) {
+					lines = append(lines, StyleDel.Render(visualLine))
+				}
 			default:
-				lines = append(lines, wrapLine(" "+diffLine.Content, width))
+				lines = append(lines, wrapLineParts(" "+diffLine.Content, width)...)
 			}
 		}
 	}
@@ -51,29 +55,32 @@ func RenderDiffView(file *internal.FileDiff, scrollOffset, width, height int) st
 
 // wrapLine wraps one rendered line by terminal cell width.
 func wrapLine(line string, width int) string {
+	return strings.Join(wrapLineParts(line, width), "\n")
+}
+
+// wrapLineParts wraps one rendered line into visual lines by terminal cell width.
+func wrapLineParts(line string, width int) []string {
 	if width <= 0 || runewidth.StringWidth(line) <= width {
-		return line
+		return []string{line}
 	}
 
-	var result strings.Builder
+	var result []string
 	first := truncateToWidth(line, width)
-	result.WriteString(first)
+	result = append(result, first)
 	remaining := line[len(first):]
 
 	for len(remaining) > 0 {
-		result.WriteString("\n")
 		prefix := "  "
 		chunkWidth := width - runewidth.StringWidth(prefix)
 		if chunkWidth <= 0 {
 			chunkWidth = 1
 		}
 		chunk := truncateToWidth(remaining, chunkWidth)
-		result.WriteString(prefix)
-		result.WriteString(chunk)
+		result = append(result, prefix+chunk)
 		remaining = remaining[len(chunk):]
 	}
 
-	return result.String()
+	return result
 }
 
 // truncateToWidth returns the longest prefix that fits within width cells.

internal/ui/diffview_test.go

@@ -3,6 +3,8 @@ package ui
 import (
 	"strings"
 	"testing"
+
+	"github.com/Astro-Han/diffpane/internal"
 )
 
 // TestWrapLineShort verifies lines shorter than the viewport stay untouched.
@@ -36,3 +38,39 @@ func TestWrapLineCJK(t *testing.T) {
 		}
 	}
 }
+
+// TestRenderDiffViewCountsWrappedDisplayLines verifies scroll and height work on
+// visual lines after wrapping, not just on raw diff entries.
+func TestRenderDiffViewCountsWrappedDisplayLines(t *testing.T) {
+	file := &internal.FileDiff{
+		Path: "long.txt",
+		Hunks: []internal.DiffHunk{{
+			Header: "@@ -0,0 +1,1 @@",
+			Lines: []internal.DiffLine{{
+				Type:    internal.LineAdd,
+				Content: strings.Repeat("a", 12),
+			}},
+		}},
+	}
+
+	firstPage := RenderDiffView(file, 0, 8, 2)
+	firstLines := strings.Split(firstPage, "\n")
+	if len(firstLines) != 2 {
+		t.Fatalf("first page line count = %d, want 2", len(firstLines))
+	}
+
+	secondPage := RenderDiffView(file, 1, 8, 2)
+	secondLines := strings.Split(secondPage, "\n")
+	if len(secondLines) != 2 {
+		t.Fatalf("second page line count = %d, want 2", len(secondLines))
+	}
+	if !strings.HasPrefix(secondLines[1], "  ") {
+		t.Fatalf("second page should include wrapped continuation, got %q", secondLines[1])
+	}
+
+	thirdPage := RenderDiffView(file, 2, 8, 2)
+	thirdLines := strings.Split(thirdPage, "\n")
+	if !strings.HasPrefix(thirdLines[0], "  ") {
+		t.Fatalf("third page should start with wrapped continuation, got %q", thirdLines[0])
+	}
+}

internal/ui/messages.go

@@ -4,6 +4,7 @@ import "github.com/Astro-Han/diffpane/internal"
 
 // FilesUpdatedMsg notifies the UI that the computed file list changed.
 type FilesUpdatedMsg struct {
+	BaselineSHA  string
 	Files        []internal.FileDiff
 	ChangedPaths []string
 }

internal/ui/model.go

@@ -17,11 +17,12 @@ type Model struct {
 	Files      []internal.FileDiff
 	CurrentIdx int
 
-	FollowOn     bool
-	ScrollOffset int
-	NewCount     int
-	NewFiles     map[string]bool
-	Notification string
+	FollowOn        bool
+	ScrollOffset    int
+	NewCount        int
+	NewFiles        map[string]bool
+	LastChangedPath string
+	Notification    string
 
 	OverlayOpen      bool
 	OverlayCursor    int
@@ -79,6 +80,10 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 }
 
 func (m Model) handleFilesUpdated(msg FilesUpdatedMsg) (tea.Model, tea.Cmd) {
+	if msg.BaselineSHA != "" && msg.BaselineSHA != m.BaselineSHA {
+		return m, nil
+	}
+
 	if m.OverlayOpen {
 		if m.PendingUpdate == nil {
 			m.PendingUpdate = &msg
@@ -112,11 +117,13 @@ func (m Model) applyFilesUpdate(msg FilesUpdatedMsg) Model {
 		m.CurrentIdx = target
 		m.ScrollOffset = 0
 		m.NewFiles = make(map[string]bool)
+		m.LastChangedPath = ""
 		m.NewCount = 0
 	} else if !m.FollowOn {
 		for _, changedPath := range msg.ChangedPaths {
 			if changedPath != currentPath {
 				m.NewFiles[changedPath] = true
+				m.LastChangedPath = changedPath
 			}
 		}
 		m.NewCount = len(m.NewFiles)
@@ -141,6 +148,7 @@ func (m Model) applyFilesUpdate(msg FilesUpdatedMsg) Model {
 		m.CurrentIdx = 0
 		m.ScrollOffset = 0
 		m.NewFiles = make(map[string]bool)
+		m.LastChangedPath = ""
 		m.NewCount = 0
 	}
 
@@ -180,6 +188,7 @@ func (m Model) handleKey(key string) (tea.Model, tea.Cmd) {
 	case "f":
 		m.FollowOn = !m.FollowOn
 		if m.FollowOn {
+			m.selectLatestPendingFile()
 			m.NewCount = 0
 			m.NewFiles = make(map[string]bool)
 		}
@@ -223,6 +232,7 @@ func (m Model) handleOverlayKey(key string) (tea.Model, tea.Cmd) {
 		return m.closeOverlay(), nil
 	case "f":
 		m.FollowOn = true
+		m.selectLatestPendingFile()
 		m.NewCount = 0
 		m.NewFiles = make(map[string]bool)
 		return m.closeOverlay(), nil
@@ -234,13 +244,33 @@ func (m Model) handleOverlayKey(key string) (tea.Model, tea.Cmd) {
 func (m Model) closeOverlay() Model {
 	m.OverlayOpen = false
 	m.OverlaySnapshot = nil
-	if m.PendingUpdate != nil {
+	if m.PendingUpdate != nil && (m.PendingUpdate.BaselineSHA == "" || m.PendingUpdate.BaselineSHA == m.BaselineSHA) {
 		m = m.applyFilesUpdate(*m.PendingUpdate)
-		m.PendingUpdate = nil
 	}
+	m.PendingUpdate = nil
 	return m
 }
 
+func (m *Model) selectLatestPendingFile() {
+	if m.LastChangedPath != "" {
+		for i, file := range m.Files {
+			if file.Path == m.LastChangedPath {
+				m.CurrentIdx = i
+				m.ScrollOffset = 0
+				return
+			}
+		}
+	}
+
+	for i := len(m.Files) - 1; i >= 0; i-- {
+		if m.NewFiles[m.Files[i].Path] {
+			m.CurrentIdx = i
+			m.ScrollOffset = 0
+			return
+		}
+	}
+}
+
 // View renders header, content, and footer into the terminal viewport.
 func (m Model) View() string {
 	if m.Width == 0 || m.Height == 0 {