From 861e1f9584e66a24baf8015129da53fa3c0c6e3e Mon Sep 17 00:00:00 2001 From: Stefan Kudev Date: Thu, 30 Apr 2026 13:51:24 +0100 Subject: [PATCH 01/11] docs: update PROBLEMS.md to remove issues which are now resolved --- docs/PROBLEMS.md | 75 +++--------------------------------------------- 1 file changed, 4 insertions(+), 71 deletions(-) diff --git a/docs/PROBLEMS.md b/docs/PROBLEMS.md index ea6d396..cfe711f 100644 --- a/docs/PROBLEMS.md +++ b/docs/PROBLEMS.md @@ -7,11 +7,10 @@ ## Table of Contents 1. [Executive Summary](#executive-summary) -2. [Critical Issues (Block Release)](#critical-issues-block-release) -3. [High-Priority Issues (Fix Before v1.0)](#high-priority-issues-fix-before-v10) -4. [Medium-Priority Issues (Follow-Up PRs)](#medium-priority-issues-follow-up-prs) -5. [Low-Priority Issues (Optional)](#low-priority-issues-optional) -6. [Epic Issues / Feature Gaps](#epic-issues--feature-gaps) +2. [High-Priority Issues (Fix Before v1.0)](#high-priority-issues-fix-before-v10) +3. [Medium-Priority Issues (Follow-Up PRs)](#medium-priority-issues-follow-up-prs) +4. [Low-Priority Issues (Optional)](#low-priority-issues-optional) +5. [Epic Issues / Feature Gaps](#epic-issues--feature-gaps) --- @@ -27,72 +26,6 @@ Beyond code quality issues, there are **6 major feature gaps** from PROBLEMS.md --- -## Critical Issues (Block Release) - -### 🔴 CRITICAL #1: Windows Compilation Broken - -**Status:** ✅ SOLVED -**Location:** `src/scanner/dir.rs:181-185` -**Severity:** CRITICAL -**Effort:** 30 minutes - -**Problem:** - -The `get_inode()` function uses Unix-only APIs: -```rust -fn get_inode(path: &Path) -> std::io::Result { - use std::os::unix::fs::MetadataExt; // ← NOT AVAILABLE ON WINDOWS - let metadata = std::fs::metadata(path)?; - Ok(metadata.ino()) -} -``` - -**Solution:** - -Added conditional compilation to provide a stable, cross-platform implementation. On Unix, it continues to use inodes. On Windows and other platforms, it returns an error, causing the scanner to gracefully skip inode-based deduplication and rely on path-based deduplication (which is already implemented via `discovered_prefixes`). - -```rust -#[cfg(unix)] -fn get_inode(path: &Path) -> std::io::Result { - use std::os::unix::fs::MetadataExt; - let metadata = std::fs::metadata(path)?; - Ok(metadata.ino()) -} - -#[cfg(not(unix))] -fn get_inode(_path: &Path) -> std::io::Result { - // On Windows, we can't use inodes. Return error or fallback to path-based dedup. - Err(std::io::Error::new( - std::io::ErrorKind::Other, - "inode tracking not supported on this platform", - )) -} -``` - ---- - -### 🔴 CRITICAL #2: Ecosystem Filtering Logic Mismatch - -**Status:** ✅ SOLVED -**Location:** `src/ui/tui.rs:115-127` vs `src/args.rs:94-110` -**Severity:** CRITICAL -**Effort:** 15 minutes - -**Problem:** - -The TUI background scan thread reimplements ecosystem filtering logic, creating code duplication and potential divergence in behavior. - -**Solution:** - -Refactored `spawn_scan_thread` in `src/ui/tui.rs` to clone the `Args` struct and call its existing `get_ecosystems()` method. This ensures that filtering logic is unified and consistent across both the TUI and plain-text modes. - -```rust -// In src/ui/tui.rs -let target_ecosystems = args_clone.get_ecosystems(&ecosystems); -``` - ---- - ## High-Priority Issues (Fix Before v1.0) ### 🟠 HIGH #3: Error Handling Divergence From 08170fafe06a67a899852ad825d09f478deb241a Mon Sep 17 00:00:00 2001 From: Stefan Kudev Date: Thu, 30 Apr 2026 13:58:35 +0100 Subject: [PATCH 02/11] fix(lib): make run() and run_plain() return Result for proper error propagation --- src/lib.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7295e80..1e8fa70 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,23 +5,25 @@ pub mod scanner; pub mod size_util; pub mod ui; +use anyhow::Context; use args::Args; use clap::Parser; use deleter::{delete_folders, print_delete_summary}; use size_util::format_size; -pub fn run() { +pub fn run() -> anyhow::Result<()> { let args = Args::parse(); if args.no_tui { - run_plain(args); + run_plain(args) } else { - ui::tui::run_tui(args).expect("TUI failed"); + ui::tui::run_tui(args) } } -pub fn run_plain(args: Args) { - let ecosystems = config::load_ecosystems().expect("failed to load ecosystems"); +pub fn run_plain(args: Args) -> anyhow::Result<()> { + let ecosystems = + config::load_ecosystems().context("Failed to load ecosystem configurations")?; let target_ecosystems = args.get_ecosystems(&ecosystems); let include_globals = args.should_include_globals(); let excluded_dirs = args.get_excluded_dirs(); @@ -80,4 +82,6 @@ pub fn run_plain(args: Args) { ); } } + + Ok(()) } From af55dcdc001e2dffbc22e27b73610adcafb585d1 Mon Sep 17 00:00:00 2001 From: Stefan Kudev Date: Thu, 30 Apr 2026 13:58:43 +0100 Subject: [PATCH 03/11] fix(main): handle run() Result and exit with error code on failure --- src/main.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index e74a28b..9452309 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,3 +1,6 @@ fn main() { - everykill::run(); + if let Err(e) = everykill::run() { + eprintln!("Error: {}", e); + std::process::exit(1); + } } From fb469669fac399e4aa43566693624ad5362fb22a Mon Sep 17 00:00:00 2001 From: Stefan Kudev Date: Thu, 30 Apr 2026 13:58:54 +0100 Subject: [PATCH 04/11] docs: remove solved HIGH #3 error handling issue from PROBLEMS.md --- docs/PROBLEMS.md | 61 ------------------------------------------------ 1 file changed, 61 deletions(-) diff --git a/docs/PROBLEMS.md b/docs/PROBLEMS.md index cfe711f..2167197 100644 --- a/docs/PROBLEMS.md +++ b/docs/PROBLEMS.md @@ -28,67 +28,6 @@ Beyond code quality issues, there are **6 major feature gaps** from PROBLEMS.md ## High-Priority Issues (Fix Before v1.0) -### 🟠 HIGH #3: Error Handling Divergence - -**Status:** ❌ UNSOLVED -**Location:** `src/lib.rs:13-20` -**Severity:** HIGH -**Effort:** 20 minutes - -**Problem:** - -The main `run()` function has inconsistent error handling: -```rust -pub fn run() { - let args = Args::parse(); - if args.no_tui { - run_plain(args); // ← Silently swallows errors - } else { - ui::tui::run_tui(args).expect("TUI failed"); // ← Can panic! - } -} -``` - -- TUI mode can panic with generic "TUI failed" message (no context) -- Plain-text mode silently swallows errors inside `run_plain()` -- `run()` itself doesn't return a Result, so callers can't handle errors gracefully - -**Example failures:** -- Corrupt ecosystem JSON → unhelpful panic -- Permission denied during deletion → silent failure -- Terminal setup fails → cryptic panic - -**Recommended Fix:** - -Make `run()` return `Result<()>` and propagate errors: -```rust -pub fn run() -> anyhow::Result<()> { - let args = Args::parse(); - if args.no_tui { - run_plain(args) - } else { - ui::tui::run_tui(args) - } -} - -pub fn run_plain(args: Args) -> anyhow::Result<()> { - let ecosystems = config::load_ecosystems() - .context("Failed to load ecosystem configurations")?; - // ... rest of function ... - Ok(()) -} - -// In main.rs: -fn main() { - if let Err(e) = everykill::run() { - eprintln!("Error: {}", e); - std::process::exit(1); - } -} -``` - ---- - ### 🟠 HIGH #4: Dangerous Panic Hook Re-wrapping **Status:** ❌ UNSOLVED From 864a964436f84acf7a5f65b8b9632fbe65ca2c9b Mon Sep 17 00:00:00 2001 From: Stefan Kudev Date: Thu, 30 Apr 2026 14:04:13 +0100 Subject: [PATCH 05/11] fix(tui): use atomic flag to prevent panic hook re-wrapping --- src/ui/tui.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/ui/tui.rs b/src/ui/tui.rs index a148c6c..d6d8e84 100644 --- a/src/ui/tui.rs +++ b/src/ui/tui.rs @@ -1,5 +1,6 @@ use std::collections::HashSet; use std::io::{self, Stdout}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::{self, Receiver}; use std::thread; use std::time::{Duration, Instant}; @@ -34,17 +35,23 @@ use crate::ui::widgets::{ // Tick rate for the event loop (~60 fps) const TICK_MS: u64 = 16; +// Guard to prevent double-wrapping of panic hook +static PANIC_HOOK_SET: AtomicBool = AtomicBool::new(false); + // --------------------------------------------------------------------------- // Public entry point // --------------------------------------------------------------------------- pub fn run_tui(args: Args) -> anyhow::Result<()> { // Install panic hook to restore terminal before printing panic details - let original_hook = std::panic::take_hook(); - std::panic::set_hook(Box::new(move |info| { - let _ = restore_terminal_raw(); - original_hook(info); - })); + // Use atomic flag to prevent re-wrapping on multiple invocations + if !PANIC_HOOK_SET.swap(true, Ordering::SeqCst) { + let original_hook = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + let _ = restore_terminal_raw(); + original_hook(info); + })); + } let mut terminal = setup_terminal()?; let result = run_app(&mut terminal, args); From c2331a81437788c005c3b700baa08730f5d36ba3 Mon Sep 17 00:00:00 2001 From: Stefan Kudev Date: Thu, 30 Apr 2026 14:04:21 +0100 Subject: [PATCH 06/11] docs: remove solved HIGH #4 panic hook issue from PROBLEMS.md --- docs/PROBLEMS.md | 48 ------------------------------------------------ 1 file changed, 48 deletions(-) diff --git a/docs/PROBLEMS.md b/docs/PROBLEMS.md index 2167197..f5330ba 100644 --- a/docs/PROBLEMS.md +++ b/docs/PROBLEMS.md @@ -28,54 +28,6 @@ Beyond code quality issues, there are **6 major feature gaps** from PROBLEMS.md ## High-Priority Issues (Fix Before v1.0) -### 🟠 HIGH #4: Dangerous Panic Hook Re-wrapping - -**Status:** ❌ UNSOLVED -**Location:** `src/ui/tui.rs:42-47` -**Severity:** HIGH -**Effort:** 20 minutes - -**Problem:** - -Panic hook is set unconditionally, risking double-wrapping on multiple invocations: - -```rust -let original_hook = std::panic::take_hook(); -std::panic::set_hook(Box::new(move |info| { - let _ = restore_terminal_raw(); - original_hook(info); -})); -``` - -**Risks:** -- If called twice, the second invocation wraps the already-wrapped hook (nesting) -- If wrapped hook panics, recursion occurs -- Terminal might not be restored if panic hook itself panics - -**Recommended Fix:** - -Use atomic flag to prevent re-wrapping: -```rust -use std::sync::atomic::{AtomicBool, Ordering}; - -static PANIC_HOOK_SET: AtomicBool = AtomicBool::new(false); - -pub fn run_tui(args: Args) -> anyhow::Result<()> { - // Only install panic hook once - if !PANIC_HOOK_SET.swap(true, Ordering::SeqCst) { - let original_hook = std::panic::take_hook(); - std::panic::set_hook(Box::new(move |info| { - let _ = restore_terminal_raw(); - original_hook(info); - })); - } - - // Rest of function... -} -``` - ---- - ### 🟠 HIGH #5: Status Messages Cleared Immediately **Status:** ❌ UNSOLVED From 2b0b4d9f3333d416d5037e020d862a60ea3e4112 Mon Sep 17 00:00:00 2001 From: Stefan Kudev Date: Thu, 30 Apr 2026 14:05:26 +0100 Subject: [PATCH 07/11] style: use sort_by_key instead of sort_by in lib.rs --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 1e8fa70..684b533 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,7 +43,7 @@ pub fn run_plain(args: Args) -> anyhow::Result<()> { let folders = match args.sort { Some(args::SortBy::Size) => { let mut folders = folders; - folders.sort_by(|a, b| b.size_bytes.cmp(&a.size_bytes)); + folders.sort_by_key(|b| std::cmp::Reverse(b.size_bytes)); folders } Some(args::SortBy::Path) => { From ceb05c069ec4e2c167dc06232524e1ac47e7a1bc Mon Sep 17 00:00:00 2001 From: Stefan Kudev Date: Thu, 30 Apr 2026 14:12:21 +0100 Subject: [PATCH 08/11] fix(ui): add timestamped status messages with 5-second auto-clear --- src/ui/app.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/ui/app.rs b/src/ui/app.rs index 75b59c3..3b2e019 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -1,5 +1,6 @@ use std::collections::HashSet; use std::path::PathBuf; +use std::time::{Duration, Instant}; use crate::config::DiscoveredFolder; @@ -66,6 +67,8 @@ pub struct AppState { /// Transient status message shown in footer (errors, dry-run output, etc.) pub status_message: Option, + /// Timestamp when status_message was set (for auto-clear) + pub status_message_timestamp: Option, } impl AppState { @@ -83,6 +86,21 @@ impl AppState { total_selected_bytes: 0, selected_count: 0, status_message: None, + status_message_timestamp: None, + } + } + + pub fn show_status(&mut self, message: String) { + self.status_message = Some(message); + self.status_message_timestamp = Some(Instant::now()); + } + + pub fn clear_expired_status(&mut self) { + if let Some(timestamp) = self.status_message_timestamp { + if timestamp.elapsed() > Duration::from_secs(5) { + self.status_message = None; + self.status_message_timestamp = None; + } } } From 8127fe6819cce48761b58383e7d540de3b6a27e7 Mon Sep 17 00:00:00 2001 From: Stefan Kudev Date: Thu, 30 Apr 2026 14:12:43 +0100 Subject: [PATCH 09/11] fix(tui): use show_status method and auto-clear status in event loop --- src/ui/tui.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ui/tui.rs b/src/ui/tui.rs index d6d8e84..4539470 100644 --- a/src/ui/tui.rs +++ b/src/ui/tui.rs @@ -169,6 +169,9 @@ fn run_app(terminal: &mut Terminal>, args: Args) -> any state.handle_scan_event(event); } + // Clear expired status messages + state.clear_expired_status(); + // Render let term_size = terminal.size().unwrap_or_default(); let term_width = term_size.width; @@ -266,9 +269,6 @@ fn render( // --------------------------------------------------------------------------- fn handle_key(state: &mut AppState, key: KeyCode, viewport_height: usize) -> bool { - // Clear transient status message on any key - state.status_message = None; - match &state.mode { AppMode::Normal => handle_key_normal(state, key, viewport_height), AppMode::FilterPopup => { @@ -302,7 +302,7 @@ fn handle_key_normal(state: &mut AppState, key: KeyCode, viewport_height: usize) } else { "Dry-run OFF — deletions are permanent".to_string() }; - state.status_message = Some(msg); +state.show_status(msg); } // Filter popup @@ -362,7 +362,7 @@ fn execute_deletion(state: &mut AppState) { let summary = delete_folders(&state.folders, state.dry_run); if state.dry_run { - state.status_message = Some(format!( + state.show_status(format!( "Dry-run: would free {} from {} folder(s)", crate::size_util::format_size(summary.freed_bytes), summary.deleted_count @@ -390,7 +390,7 @@ fn execute_deletion(state: &mut AppState) { summary.errors.len() ) }; - state.status_message = Some(msg); + state.show_status(msg); } } From 7505c40ca22673b9e06807266dc85eef7616250c Mon Sep 17 00:00:00 2001 From: Stefan Kudev Date: Thu, 30 Apr 2026 14:13:01 +0100 Subject: [PATCH 10/11] docs: remove solved HIGH #5 status message issue from PROBLEMS.md --- docs/PROBLEMS.md | 72 ------------------------------------------------ 1 file changed, 72 deletions(-) diff --git a/docs/PROBLEMS.md b/docs/PROBLEMS.md index f5330ba..ee71ffb 100644 --- a/docs/PROBLEMS.md +++ b/docs/PROBLEMS.md @@ -26,78 +26,6 @@ Beyond code quality issues, there are **6 major feature gaps** from PROBLEMS.md --- -## High-Priority Issues (Fix Before v1.0) - -### 🟠 HIGH #5: Status Messages Cleared Immediately - -**Status:** ❌ UNSOLVED -**Location:** `src/ui/tui.rs:281-283` -**Severity:** HIGH -**Effort:** 30 minutes - -**Problem:** - -Every keystroke immediately clears the status message: -```rust -fn handle_key(state: &mut AppState, key: KeyCode, viewport_height: usize) -> bool { - state.status_message = None; // ← CLEARED ON EVERY KEY! - match &state.mode { ... } -} -``` - -**What happens:** -1. User deletes folders → status shows "Deleted 5 folders, freed 250MB" -2. User presses any key (even arrow) → status is IMMEDIATELY cleared -3. User can't read the completion message - -**Expected behavior:** -- Status persists for 1-2 seconds then auto-clears -- User has time to read results - -**Recommended Fix:** - -Implement timeout-based clearing with timestamp: -```rust -use std::time::Instant; - -pub struct AppState { - pub status_message: Option, - pub status_message_timestamp: Option, // Add this - // ... rest of fields ... -} - -impl AppState { - pub fn show_status(&mut self, message: String) { - self.status_message = Some(message); - self.status_message_timestamp = Some(Instant::now()); - } - - pub fn clear_expired_status(&mut self) { - if let Some(timestamp) = self.status_message_timestamp { - if timestamp.elapsed() > Duration::from_secs(2) { - self.status_message = None; - self.status_message_timestamp = None; - } - } - } -} - -// In handle_key: REMOVE the auto-clear -fn handle_key(state: &mut AppState, key: KeyCode, viewport_height: usize) -> bool { - // REMOVED: state.status_message = None; - match &state.mode { ... } -} - -// In render loop: auto-clear after 2 seconds -while !should_quit { - // ... handle events ... - state.clear_expired_status(); - terminal.draw(|frame| { ... })?; -} -``` - ---- - ## Medium-Priority Issues (Follow-Up PRs) ### 🟡 MEDIUM #6: Integer Overflow in Size Calculation From fb0b9d0357e7693bd0bb1f76a43b08f09562828a Mon Sep 17 00:00:00 2001 From: Stefan Kudev Date: Thu, 30 Apr 2026 15:28:41 +0100 Subject: [PATCH 11/11] style: fix rustfmt formatting in tui.rs --- src/ui/tui.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/tui.rs b/src/ui/tui.rs index 4539470..2257174 100644 --- a/src/ui/tui.rs +++ b/src/ui/tui.rs @@ -302,7 +302,7 @@ fn handle_key_normal(state: &mut AppState, key: KeyCode, viewport_height: usize) } else { "Dry-run OFF — deletions are permanent".to_string() }; -state.show_status(msg); + state.show_status(msg); } // Filter popup