Skip to content

Commit 9fbadfd

Browse files
committed
refactor: fix review findings from V1.32.0 code review
- analyze: skip zero-size entries in Up/Down navigation so cursor never lands on a hidden row - status: consolidate battery health logic; calculateHealthScore now delegates to batteryHealthLabel instead of reimplementing thresholds - analyze: remove dead "System Caches" case and merge duplicate 💾 arms in insightIcon; update test to match - analyze: remove redundant cacheBreakdownPaths alias, iterate cleanablePaths directly - analyze: simplify measureInsightSize signature to accept path string; clean up scanOverviewPathCmd call site - clean: wrap bun cache-path spinner in TTY guard, consistent with the surrounding code
1 parent 81c36e4 commit 9fbadfd

File tree

5 files changed

+28
-35
lines changed

5 files changed

+28
-35
lines changed

cmd/analyze/insights.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,7 @@ func createInsightEntries() []dirEntry {
6767
{"Gradle Cache", filepath.Join(home, ".gradle", "caches")},
6868
{"CocoaPods Cache", filepath.Join(home, "Library", "Caches", "CocoaPods")},
6969
}
70-
cacheBreakdownPaths := cleanablePaths
71-
for _, c := range cacheBreakdownPaths {
70+
for _, c := range cleanablePaths {
7271
if info, err := os.Stat(c.path); err == nil && info.IsDir() {
7372
entries = append(entries, dirEntry{
7473
Name: c.name,
@@ -82,18 +81,16 @@ func createInsightEntries() []dirEntry {
8281
return entries
8382
}
8483

85-
// measureInsightSize measures the size of an insight entry.
86-
// Some insights need special measurement (e.g., Old Downloads only counts old files).
87-
func measureInsightSize(entry dirEntry) (int64, error) {
84+
// measureInsightSize measures the size of a path.
85+
// Old Downloads is treated specially: only files older than 90 days are counted.
86+
func measureInsightSize(path string) (int64, error) {
8887
home := os.Getenv("HOME")
8988

90-
// Old Downloads: only count files older than 90 days.
91-
if home != "" && entry.Path == filepath.Join(home, "Downloads") {
92-
return measureOldDownloads(entry.Path, 90)
89+
if home != "" && path == filepath.Join(home, "Downloads") {
90+
return measureOldDownloads(path, 90)
9391
}
9492

95-
// All others: standard directory size measurement.
96-
return measureOverviewSize(entry.Path)
93+
return measureOverviewSize(path)
9794
}
9895

9996
// measureOldDownloads calculates total size of files in a directory
@@ -140,16 +137,14 @@ func insightIcon(entry dirEntry) string {
140137
return "📱"
141138
case "Old Downloads (90d+)":
142139
return "📥"
143-
case "System Caches", "Homebrew Cache", "pip Cache", "CocoaPods Cache", "Gradle Cache":
140+
case "Homebrew Cache", "pip Cache", "CocoaPods Cache", "Gradle Cache", "Spotify Cache", "JetBrains Cache":
144141
return "💾"
145142
case "System Logs":
146143
return "📋"
147144
case "Xcode DerivedData", "Xcode Archives":
148145
return "🔨"
149146
case "Xcode Simulators":
150147
return "📲"
151-
case "Spotify Cache", "JetBrains Cache":
152-
return "💾"
153148
case "Docker Data":
154149
return "🐳"
155150
default:

cmd/analyze/insights_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func TestInsightIcon(t *testing.T) {
4141
}{
4242
{"iOS Backups", "📱"},
4343
{"Old Downloads (90d+)", "📥"},
44-
{"System Caches", "💾"},
44+
{"Homebrew Cache", "💾"},
4545
{"System Logs", "📋"},
4646
{"Xcode Simulators", "📲"},
4747
{"Docker Data", "🐳"},
@@ -99,7 +99,7 @@ func TestMeasureInsightSizeFallsBackToOverview(t *testing.T) {
9999
t.Fatal(err)
100100
}
101101

102-
size, err := measureInsightSize(dirEntry{Path: dir})
102+
size, err := measureInsightSize(dir)
103103
if err != nil {
104104
t.Fatalf("measureInsightSize: %v", err)
105105
}

cmd/analyze/main.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,11 @@ func (m model) updateKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
640640
}
641641
}
642642
} else if len(m.entries) > 0 && m.selected > 0 {
643-
m.selected--
643+
next := m.selected - 1
644+
for next > 0 && m.entries[next].Size == 0 {
645+
next--
646+
}
647+
m.selected = next
644648
if m.selected < m.offset {
645649
m.offset = m.selected
646650
}
@@ -655,7 +659,11 @@ func (m model) updateKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
655659
}
656660
}
657661
} else if len(m.entries) > 0 && m.selected < len(m.entries)-1 {
658-
m.selected++
662+
next := m.selected + 1
663+
for next < len(m.entries)-1 && m.entries[next].Size == 0 {
664+
next++
665+
}
666+
m.selected = next
659667
viewport := calculateViewport(m.height, false)
660668
if m.selected >= m.offset+viewport {
661669
m.offset = m.selected - viewport + 1
@@ -1157,11 +1165,7 @@ func (m *model) removePathFromView(path string) {
11571165

11581166
func scanOverviewPathCmd(path string, index int) tea.Cmd {
11591167
return func() tea.Msg {
1160-
var size int64
1161-
var err error
1162-
1163-
size, err = measureInsightSize(dirEntry{Path: path})
1164-
1168+
size, err := measureInsightSize(path)
11651169
return overviewSizeMsg{
11661170
Path: path,
11671171
Index: index,

cmd/status/metrics_health.go

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -138,18 +138,12 @@ func calculateHealthScore(cpu CPUStatus, mem MemoryStatus, disks []DiskStatus, d
138138
// Battery health penalty (only when battery present).
139139
if len(batteries) > 0 {
140140
b := batteries[0]
141-
if b.CycleCount > batteryCycleDanger {
141+
_, sev := batteryHealthLabel(b.CycleCount, b.Capacity)
142+
switch sev {
143+
case "danger":
142144
score -= 5
143-
issues = append(issues, "Battery Aging")
144-
} else if b.CycleCount > batteryCycleWarn {
145-
score -= 2
146-
}
147-
if b.Capacity > 0 && b.Capacity < batteryCapDanger {
148-
score -= 5
149-
if b.CycleCount <= batteryCycleDanger {
150-
issues = append(issues, "Battery Degraded")
151-
}
152-
} else if b.Capacity > 0 && b.Capacity < batteryCapWarn {
145+
issues = append(issues, "Battery Service Soon")
146+
case "warn":
153147
score -= 2
154148
}
155149
}

lib/clean/dev.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,9 @@ clean_dev_npm() {
110110
bun_cache_cleaned=true
111111
fi
112112

113-
start_section_spinner "Checking bun cache path..."
113+
if [[ -t 1 ]]; then start_section_spinner "Checking bun cache path..."; fi
114114
bun_cache_path=$(run_with_timeout 2 bun pm cache 2> /dev/null) || bun_cache_path=""
115-
stop_section_spinner
115+
if [[ -t 1 ]]; then stop_section_spinner; fi
116116

117117
if [[ -z "$bun_cache_path" || "$bun_cache_path" != /* ]]; then
118118
bun_cache_path="$bun_default_cache"

0 commit comments

Comments
 (0)