Skip to content

Commit 397c895

Browse files
committed
Fix all golangci-lint warnings
- Extract writeStagedBlameFile() to reduce nestif complexity in blame.go - Combine param types in FileBlame signature (gocritic) - Handle errcheck/gosec on temp file cleanup paths - Extract deletePlaceholderVisualHeight() to reduce nestif in annotate.go - Remove stale //nolint:gosec directives from diff.go and diff_test.go - Pre-allocate slices in directory.go, fallback.go, theme.go, filetree.go, mdtoc.go, and model_test.go; preserve nil-vs-empty semantics in fallback.go
1 parent 24365bf commit 397c895

11 files changed

Lines changed: 58 additions & 40 deletions

File tree

diff/blame.go

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,28 +19,14 @@ type BlameLine struct {
1919
// For unstaged single-ref diffs this is the worktree; for two-ref diffs this is the target ref.
2020
// For staged diffs this is the index snapshot. The returned map is keyed by 1-based line
2121
// number (matching DiffLine.NewNum).
22-
func (g *Git) FileBlame(ref string, file string, staged bool) (map[int]BlameLine, error) {
22+
func (g *Git) FileBlame(ref, file string, staged bool) (map[int]BlameLine, error) {
2323
args := []string{"blame", "--line-porcelain"}
2424
if staged {
25-
indexContent, err := g.runGit("show", ":"+file)
25+
tmpName, err := g.writeStagedBlameFile(file)
2626
if err != nil {
27-
return nil, fmt.Errorf("read index contents for %s: %w", file, err)
27+
return nil, err
2828
}
29-
30-
tmp, err := os.CreateTemp("", "revdiff-blame-*")
31-
if err != nil {
32-
return nil, fmt.Errorf("create temp blame file for %s: %w", file, err)
33-
}
34-
tmpName := tmp.Name()
35-
defer os.Remove(tmpName)
36-
if _, err := tmp.WriteString(indexContent); err != nil {
37-
tmp.Close()
38-
return nil, fmt.Errorf("write temp blame file for %s: %w", file, err)
39-
}
40-
if err := tmp.Close(); err != nil {
41-
return nil, fmt.Errorf("close temp blame file for %s: %w", file, err)
42-
}
43-
29+
defer os.Remove(tmpName) //nolint:errcheck // best-effort temp file cleanup
4430
args = append(args, "--contents", tmpName)
4531
} else if targetRef := blameTargetRef(ref); targetRef != "" {
4632
args = append(args, targetRef)
@@ -53,6 +39,27 @@ func (g *Git) FileBlame(ref string, file string, staged bool) (map[int]BlameLine
5339
return parseBlame(out)
5440
}
5541

42+
// writeStagedBlameFile writes the staged (index) contents of file to a temp file
43+
// and returns its path. The caller is responsible for removing the temp file.
44+
func (g *Git) writeStagedBlameFile(file string) (string, error) {
45+
indexContent, err := g.runGit("show", ":"+file)
46+
if err != nil {
47+
return "", fmt.Errorf("read index contents for %s: %w", file, err)
48+
}
49+
tmp, err := os.CreateTemp("", "revdiff-blame-*")
50+
if err != nil {
51+
return "", fmt.Errorf("create temp blame file for %s: %w", file, err)
52+
}
53+
if _, err := tmp.WriteString(indexContent); err != nil {
54+
tmp.Close() //nolint:gosec // best-effort close in error path
55+
return "", fmt.Errorf("write temp blame file for %s: %w", file, err)
56+
}
57+
if err := tmp.Close(); err != nil {
58+
return "", fmt.Errorf("close temp blame file for %s: %w", file, err)
59+
}
60+
return tmp.Name(), nil
61+
}
62+
5663
func blameTargetRef(ref string) string {
5764
parts := strings.Split(ref, "..")
5865
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {

diff/diff.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func (g *Git) diffArgs(ref string, staged bool) []string {
9292

9393
// runGit executes a git command in the working directory and returns its output.
9494
func (g *Git) runGit(args ...string) (string, error) {
95-
cmd := exec.CommandContext(context.Background(), "git", args...) //nolint:gosec // git args are constructed internally
95+
cmd := exec.CommandContext(context.Background(), "git", args...)
9696
cmd.Dir = g.workDir
9797
out, err := cmd.Output()
9898
if err != nil {

diff/diff_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ func writeFile(t *testing.T, dir, name, content string) {
325325

326326
func gitCmd(t *testing.T, dir string, args ...string) {
327327
t.Helper()
328-
cmd := exec.Command("git", args...) //nolint:gosec // test helper
328+
cmd := exec.Command("git", args...)
329329
cmd.Dir = dir
330330
out, err := cmd.CombinedOutput()
331331
require.NoError(t, err, "git %v failed: %s", args, string(out))

diff/directory.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func (dr *DirectoryReader) ChangedFiles(_ string, _ bool) ([]string, error) {
4040
return nil, fmt.Errorf("git ls-files: %w", err)
4141
}
4242

43-
var files []string
43+
files := make([]string, 0, strings.Count(string(out), "\x00"))
4444
for entry := range strings.SplitSeq(string(out), "\x00") {
4545
if entry == "" {
4646
continue

diff/fallback.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,20 @@ func NewFileReader(files []string, workDir string) *FileReader {
160160

161161
// ChangedFiles returns the file list, resolved against workDir, filtered to only those that exist on disk.
162162
func (r *FileReader) ChangedFiles(_ string, _ bool) ([]string, error) {
163-
var result []string
163+
if len(r.files) == 0 {
164+
return nil, nil
165+
}
166+
result := make([]string, 0, len(r.files))
164167
for _, f := range r.files {
165168
resolved := resolvePath(r.workDir, f)
166169
if _, err := os.Stat(resolved); err != nil {
167170
continue // skip files that don't exist
168171
}
169172
result = append(result, resolved)
170173
}
174+
if len(result) == 0 {
175+
return nil, nil
176+
}
171177
return result, nil
172178
}
173179

theme/theme.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ func List(themesDir string) ([]string, error) {
223223
return nil, fmt.Errorf("reading themes dir: %w", err)
224224
}
225225

226-
var names []string
226+
names := make([]string, 0, len(entries))
227227
for _, e := range entries {
228228
if e.IsDir() {
229229
continue

ui/annotate.go

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -285,20 +285,7 @@ func (m Model) cursorViewportY() int {
285285
// delete-only placeholders render synthetic text ("⋯ N lines deleted"), not original content.
286286
// use placeholder text for wrapping to stay in sync with renderDeletePlaceholder.
287287
if m.isDeleteOnlyPlaceholder(i, hunks) {
288-
if m.wrapMode {
289-
text := m.deletePlaceholderText(i)
290-
gutterExtra := 0
291-
if m.lineNumbers {
292-
gutterExtra = m.lineNumGutterWidth()
293-
}
294-
if m.hasBlameGutter() {
295-
gutterExtra += m.blameGutterWidth()
296-
}
297-
wrapWidth := m.diffContentWidth() - wrapGutterWidth - gutterExtra
298-
y += len(m.wrapContent(text, wrapWidth))
299-
} else {
300-
y++ // placeholder is always 1 row when not wrapping
301-
}
288+
y += m.deletePlaceholderVisualHeight(i)
302289
continue
303290
}
304291
y += m.wrappedLineCount(i) // the diff line (may occupy multiple visual rows when wrapping)

ui/collapsed.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,24 @@ func (m Model) deletePlaceholderText(hunkStart int) string {
173173
return fmt.Sprintf("⋯ %d lines deleted", count)
174174
}
175175

176+
// deletePlaceholderVisualHeight returns the number of visual rows a delete-only placeholder
177+
// occupies, accounting for wrap mode and gutter widths.
178+
func (m Model) deletePlaceholderVisualHeight(hunkStart int) int {
179+
if !m.wrapMode {
180+
return 1
181+
}
182+
text := m.deletePlaceholderText(hunkStart)
183+
gutterExtra := 0
184+
if m.lineNumbers {
185+
gutterExtra = m.lineNumGutterWidth()
186+
}
187+
if m.hasBlameGutter() {
188+
gutterExtra += m.blameGutterWidth()
189+
}
190+
wrapWidth := m.diffContentWidth() - wrapGutterWidth - gutterExtra
191+
return len(m.wrapContent(text, wrapWidth))
192+
}
193+
176194
// renderDeletePlaceholder renders a placeholder line for a delete-only hunk in collapsed mode.
177195
// shows "⋯ N lines deleted" with remove styling so users know deletions exist and can expand with '.'.
178196
// when search is active, matching placeholders use search highlight instead of remove styling.

ui/filetree.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func (ft *fileTree) buildEntries(files []string) []treeEntry {
5757
}
5858
sort.Strings(dirs)
5959

60-
var entries []treeEntry
60+
entries := make([]treeEntry, 0, len(dirs)+len(files))
6161
for _, dir := range dirs {
6262
// add directory entry
6363
dirName := dir

ui/mdtoc.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ type mdTOC struct {
2828
// fence tracking is CommonMark-compliant: closing fence must use the same character
2929
// with at least the same length as the opening fence.
3030
func parseTOC(lines []diff.DiffLine, filename string) *mdTOC {
31-
var entries []tocEntry
31+
entries := make([]tocEntry, 0, len(lines))
3232
var fenceChar rune // 0 when outside code block, '`' or '~' when inside
3333
var fenceLen int // length of the opening fence sequence
3434

0 commit comments

Comments
 (0)