Fix blame review follow-ups and persist view modes

Author: melonamin

.claude-plugin/skills/revdiff/references/config.md

@@ -26,6 +26,7 @@ Then uncomment and edit the values you want to change.
 | `--wrap` | `REVDIFF_WRAP` | Enable line wrapping in diff view | `false` |
 | `--collapsed` | `REVDIFF_COLLAPSED` | Start in collapsed diff mode | `false` |
 | `--line-numbers` | `REVDIFF_LINE_NUMBERS` | Show line numbers in diff gutter | `false` |
+| `--blame` | `REVDIFF_BLAME` | Show git blame gutter on startup | `false` |
 | `--no-confirm-discard` | `REVDIFF_NO_CONFIRM_DISCARD` | Skip confirmation when discarding annotations with Q | `false` |
 | `--chroma-style` | `REVDIFF_CHROMA_STYLE` | Chroma color theme for syntax highlighting | `catppuccin-macchiato` |
 | `--theme` | `REVDIFF_THEME` | Load color theme from `~/.config/revdiff/themes/` | |

README.md

@@ -165,6 +165,7 @@ Positional arguments support several forms:
 | `--wrap` | Enable line wrapping in diff view, env: `REVDIFF_WRAP` | `false` |
 | `--collapsed` | Start in collapsed diff mode, env: `REVDIFF_COLLAPSED` | `false` |
 | `--line-numbers` | Show line numbers in diff gutter, env: `REVDIFF_LINE_NUMBERS` | `false` |
+| `--blame` | Show git blame gutter on startup, env: `REVDIFF_BLAME` | `false` |
 | `--no-confirm-discard` | Skip confirmation when discarding annotations with Q, env: `REVDIFF_NO_CONFIRM_DISCARD` | `false` |
 | `--chroma-style` | Chroma color theme for syntax highlighting, env: `REVDIFF_CHROMA_STYLE` | `catppuccin-macchiato` |
 | `--theme` | Load color theme from `~/.config/revdiff/themes/`, env: `REVDIFF_THEME` | |

cmd/revdiff/main.go

@@ -37,6 +37,7 @@ type options struct {
 	Wrap             bool     `long:"wrap" ini-name:"wrap" env:"REVDIFF_WRAP" description:"enable line wrapping in diff view"`
 	Collapsed        bool     `long:"collapsed" ini-name:"collapsed" env:"REVDIFF_COLLAPSED" description:"start in collapsed diff mode"`
 	LineNumbers      bool     `long:"line-numbers" ini-name:"line-numbers" env:"REVDIFF_LINE_NUMBERS" description:"show line numbers in diff gutter"`
+	Blame            bool     `long:"blame" ini-name:"blame" env:"REVDIFF_BLAME" description:"show git blame gutter on startup"`
 	ChromaStyle      string   `long:"chroma-style" ini-name:"chroma-style" env:"REVDIFF_CHROMA_STYLE" default:"catppuccin-macchiato" description:"chroma style for syntax highlighting"`
 	AllFiles         bool     `long:"all-files" short:"A" no-ini:"true" description:"browse all git-tracked files, not just diffs"`
 	Exclude          []string `long:"exclude" short:"X" ini-name:"exclude" env:"REVDIFF_EXCLUDE" env-delim:"," description:"exclude files matching prefix (may be repeated)"`
@@ -296,6 +297,7 @@ func run(opts options) error {
 		Wrap:             opts.Wrap,
 		Collapsed:        opts.Collapsed,
 		LineNumbers:      opts.LineNumbers,
+		ShowBlame:        opts.Blame,
 		TabWidth:         opts.TabWidth,
 		Ref:              opts.ref(),
 		Staged:           opts.Staged,

cmd/revdiff/main_test.go

@@ -35,6 +35,8 @@ func TestParseArgs_Defaults(t *testing.T) {
 	assert.False(t, opts.NoConfirmDiscard)
 	assert.False(t, opts.Wrap)
 	assert.False(t, opts.Collapsed)
+	assert.False(t, opts.LineNumbers)
+	assert.False(t, opts.Blame)
 	assert.Empty(t, opts.Output)
 	assert.Empty(t, opts.Refs.Base)
 	assert.Empty(t, opts.Refs.Against)
@@ -140,6 +142,32 @@ func TestParseArgs_LineNumbers(t *testing.T) {
 	})
 }
 
+func TestParseArgs_Blame(t *testing.T) {
+	t.Run("flag", func(t *testing.T) {
+		opts, err := parseArgs(append(noConfigArgs(t), "--blame"))
+		require.NoError(t, err)
+		assert.True(t, opts.Blame)
+	})
+
+	t.Run("env", func(t *testing.T) {
+		t.Setenv("REVDIFF_BLAME", "true")
+		opts, err := parseArgs(noConfigArgs(t))
+		require.NoError(t, err)
+		assert.True(t, opts.Blame)
+	})
+
+	t.Run("config file", func(t *testing.T) {
+		cfgDir := t.TempDir()
+		cfgPath := filepath.Join(cfgDir, "config")
+		err := os.WriteFile(cfgPath, []byte("[Application Options]\nblame = true\n"), 0o600)
+		require.NoError(t, err)
+		opts, err := parseArgs([]string{"--config", cfgPath})
+		require.NoError(t, err)
+		assert.True(t, opts.Blame)
+	})
+}
+
+
 func TestParseArgs_OutputFlag(t *testing.T) {
 	opts, err := parseArgs([]string{"-o", "/tmp/out.txt"})
 	require.NoError(t, err)

diff/blame.go

@@ -26,7 +26,7 @@ func (g *Git) FileBlame(ref, file string, staged bool) (map[int]BlameLine, error
 		if err != nil {
 			return nil, err
 		}
-		defer os.Remove(tmpName) //nolint:errcheck // best-effort temp file cleanup
+		defer func() { _ = os.Remove(tmpName) }()
 		args = append(args, "--contents", tmpName)
 	} else if targetRef := blameTargetRef(ref); targetRef != "" {
 		args = append(args, targetRef)
@@ -51,7 +51,7 @@ func (g *Git) writeStagedBlameFile(file string) (string, error) {
 		return "", fmt.Errorf("create temp blame file for %s: %w", file, err)
 	}
 	if _, err := tmp.WriteString(indexContent); err != nil {
-		tmp.Close() //nolint:gosec // best-effort close in error path
+		_ = tmp.Close()
 		return "", fmt.Errorf("write temp blame file for %s: %w", file, err)
 	}
 	if err := tmp.Close(); err != nil {

ui/collapsed.go

@@ -180,14 +180,7 @@ func (m Model) deletePlaceholderVisualHeight(hunkStart int) int {
 		return 1
 	}
 	text := m.deletePlaceholderText(hunkStart)
-	gutterExtra := 0
-	if m.lineNumbers {
-		gutterExtra = m.lineNumGutterWidth()
-	}
-	if m.hasBlameGutter() {
-		gutterExtra += m.blameGutterWidth()
-	}
-	wrapWidth := m.diffContentWidth() - wrapGutterWidth - gutterExtra
+	wrapWidth := m.diffContentWidth() - wrapGutterWidth - m.gutterExtra()
 	return len(m.wrapContent(text, wrapWidth))
 }
 

ui/mocks/blamer.go

@@ -2,6 +2,7 @@ package ui
 
 //go:generate moq -out mocks/renderer.go -pkg mocks -skip-ensure -fmt goimports . Renderer
 //go:generate moq -out mocks/syntax_highlighter.go -pkg mocks -skip-ensure -fmt goimports . SyntaxHighlighter
+//go:generate moq -out mocks/blamer.go -pkg mocks -skip-ensure -fmt goimports . Blamer
 
 import (
 	"fmt"
@@ -97,7 +98,7 @@ type Model struct {
 	showBlame      bool                   // true when blame gutter is shown
 	blameData      map[int]diff.BlameLine // blame info keyed by 1-based new line number
 	blameAuthorLen int                    // max author display width for blame gutter
-	blameNow       time.Time             // snapshot of time.Now() set once per render pass for blame age
+	blameNow       time.Time              // snapshot of time.Now() set once per render pass for blame age
 
 	searching      bool            // true when search textinput is active (typing)
 	searchTerm     string          // last submitted search query
@@ -154,6 +155,7 @@ type ModelConfig struct {
 	Wrap             bool           // enable line wrapping
 	Collapsed        bool           // start in collapsed diff mode
 	LineNumbers      bool           // show line numbers in diff gutter
+	ShowBlame        bool           // show blame gutter on startup when available
 	Only             []string       // show only these files (match by exact path or path suffix)
 	WorkDir          string         // working directory for resolving absolute --only paths
 	Keymap           *keymap.Keymap // custom key bindings (nil uses defaults)
@@ -193,6 +195,7 @@ func NewModel(renderer Renderer, store *annotation.Store, highlighter SyntaxHigh
 		wrapMode:         cfg.Wrap,
 		lineNumbers:      cfg.LineNumbers,
 		collapsed:        collapsedState{enabled: cfg.Collapsed},
+		showBlame:        cfg.ShowBlame && cfg.Blamer != nil,
 		focus:            paneTree,
 		treeWidthRatio:   cfg.TreeWidthRatio,
 		tabSpaces:        strings.Repeat(" ", cfg.TabWidth),
@@ -1235,7 +1238,7 @@ func (m Model) statusModeIcons() string {
 		{"≋", len(m.searchMatches) > 0},
 		{"⊟", m.treeHidden},
 		{"#", m.lineNumbers},
-		{"@", m.showBlame},
+		{"b", m.showBlame},
 	}
 
 	statusFg := m.styles.colors.Muted

ui/model.go

@@ -452,6 +452,33 @@ func TestModel_LineNumbersFromConfig(t *testing.T) {
 	})
 }
 
+func TestModel_BlameFromConfig(t *testing.T) {
+	renderer := &mocks.RendererMock{
+		ChangedFilesFunc: func(string, bool) ([]string, error) { return nil, nil },
+		FileDiffFunc:     func(string, string, bool) ([]diff.DiffLine, error) { return nil, nil },
+	}
+	store := annotation.NewStore()
+	blamer := &mocks.BlamerMock{
+		FileBlameFunc: func(string, string, bool) (map[int]diff.BlameLine, error) { return map[int]diff.BlameLine{}, nil },
+	}
+
+	t.Run("blame enabled via config when blamer is available", func(t *testing.T) {
+		m := NewModel(renderer, store, noopHighlighter(), ModelConfig{ShowBlame: true, Blamer: blamer, TreeWidthRatio: 2})
+		assert.True(t, m.showBlame)
+	})
+
+	t.Run("blame disabled without blamer even if requested", func(t *testing.T) {
+		m := NewModel(renderer, store, noopHighlighter(), ModelConfig{ShowBlame: true, TreeWidthRatio: 2})
+		assert.False(t, m.showBlame)
+	})
+
+	t.Run("blame disabled by default", func(t *testing.T) {
+		m := NewModel(renderer, store, noopHighlighter(), ModelConfig{Blamer: blamer, TreeWidthRatio: 2})
+		assert.False(t, m.showBlame)
+	})
+}
+
+
 func TestModel_StatusModeIcons(t *testing.T) {
 	t.Run("all icons always present", func(t *testing.T) {
 		m := testModel(nil, nil)
@@ -6970,6 +6997,14 @@ func TestModel_StatusModeIconsLineNumbers(t *testing.T) {
 	assert.Contains(t, icons, "#")
 }
 
+func TestModel_StatusModeIconsBlame(t *testing.T) {
+	m := testModel(nil, nil)
+	m.showBlame = true
+	icons := m.statusModeIcons()
+	assert.Contains(t, icons, "b")
+	assert.NotContains(t, icons, "@")
+}
+
 func TestModel_HelpOverlayContainsLineNumbers(t *testing.T) {
 	m := testModel(nil, nil)
 	m.width = 120