diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bd4b0c9..7f880a7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: name: Check runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Install Rust uses: dtolnay/rust-toolchain@stable - name: Cargo Check @@ -24,7 +24,7 @@ jobs: name: Test Suite runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Install Rust uses: dtolnay/rust-toolchain@stable - name: Run Tests @@ -34,7 +34,7 @@ jobs: name: Rustfmt runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Install Rust uses: dtolnay/rust-toolchain@stable with: @@ -46,7 +46,7 @@ jobs: name: Clippy runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Install Rust uses: dtolnay/rust-toolchain@stable with: diff --git a/Cargo.toml b/Cargo.toml index 379151f..e6bf527 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,14 +8,15 @@ license = "MIT" [dependencies] anyhow = "1.0" clap = { version = "4.5", features = ["derive"] } -crossterm = "0.27" -dirs = "5.0" +crossterm = "0.29" +dirs = "6.0" glob = "0.3" -ratatui = "0.29" +ratatui = "0.30" rayon = "1.8" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" walkdir = "2.4" +trash = "5" [dev-dependencies] tempfile = "3.10" diff --git a/assets/everykill_logo/everykill.png b/assets/everykill_logo/everykill.png new file mode 100644 index 0000000..d749035 Binary files /dev/null and b/assets/everykill_logo/everykill.png differ diff --git a/assets/everykill_logo/everykill.psd b/assets/everykill_logo/everykill.psd new file mode 100644 index 0000000..bf011b7 Binary files /dev/null and b/assets/everykill_logo/everykill.psd differ diff --git a/assets/everykill_logo/everykill_dark.png b/assets/everykill_logo/everykill_dark.png new file mode 100644 index 0000000..a638646 Binary files /dev/null and b/assets/everykill_logo/everykill_dark.png differ diff --git a/assets/everykill_logo/variants/everykill_dark.png b/assets/everykill_logo/variants/everykill_dark.png new file mode 100644 index 0000000..a638646 Binary files /dev/null and b/assets/everykill_logo/variants/everykill_dark.png differ diff --git a/docs/PROBLEMS.md b/docs/PROBLEMS.md index ee71ffb..25260a9 100644 --- a/docs/PROBLEMS.md +++ b/docs/PROBLEMS.md @@ -7,10 +7,8 @@ ## Table of Contents 1. [Executive Summary](#executive-summary) -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) +2. [Low-Priority Issues (Optional)](#low-priority-issues-optional) +3. [Epic Issues / Feature Gaps](#epic-issues--feature-gaps) --- @@ -18,226 +16,12 @@ The everykill project demonstrates **excellent software engineering practices** with a well-architected Rust codebase, comprehensive testing (47 unit tests), and smart algorithms. All **critical issues** have been resolved for the v1.0 release. -Additionally, **3 high-priority issues** affect error handling and UX, and **11 medium-priority issues** need attention for robustness. - Beyond code quality issues, there are **6 major feature gaps** from PROBLEMS.md that need to be addressed for a complete v1.0 release (CI/CD pipelines, binary building, ecosystem markers, marketing). **With ~2 hours of focused work** on critical/high items + **addressing the epic features**, this project will be ready for v1.0. --- -## Medium-Priority Issues (Follow-Up PRs) - -### 🟡 MEDIUM #6: Integer Overflow in Size Calculation - -**Status:** ❌ UNSOLVED -**Location:** `src/scanner/size.rs` -**Severity:** MEDIUM -**Effort:** 10 minutes - -**Problem:** Size accumulation uses `u64` (max ~18.4 EB) without overflow checking. - -**Risk:** Very low in practice (modern disks ~100TB), but theoretically unsound. - -**Fix:** Use `checked_add()` with error handling. - ---- - -### 🟡 MEDIUM #7: Hardcoded SKIP_DIRS Not Configurable - -**Status:** ❌ UNSOLVED -**Location:** `src/scanner/dir.rs:5` -**Severity:** MEDIUM -**Effort:** 15 minutes - -**Problem:** -```rust -const SKIP_DIRS: &[&str] = &[".git", ".svn", ".hg", "node_modules/.cache", ".cache"]; -``` - -Can't override `.git`, `.svn`, etc. even with `--exclude` flag. What if user wants to scan inside version control dirs for a size audit? - -**Fix:** Add `--no-skip-hidden` flag or merge with `--exclude` logic. - ---- - -### 🟡 MEDIUM #8: Background Thread Termination Is Silent - -**Status:** ❌ UNSOLVED -**Location:** `src/ui/tui.rs:140-144` -**Severity:** MEDIUM -**Effort:** 5 minutes - -**Problem:** Thread exits with no logging if receiver drops. - -**Impact:** Hard to debug if thread exits unexpectedly. - -**Fix:** Add `eprintln!()` to log reason. - ---- - -### 🟡 MEDIUM #9: Unused Parameter in Render Function - -**Status:** ❌ UNSOLVED -**Location:** `src/ui/tui.rs:251` -**Severity:** MEDIUM -**Effort:** 5 minutes - -**Problem:** `_viewport_height` parameter is accepted but never used. - -**Fix:** Remove if unused, or implement if future optimization planned. - ---- - -### 🟡 MEDIUM #10: Mouse Click Bounds Checking Incomplete - -**Status:** ❌ UNSOLVED -**Location:** `src/ui/tui.rs:433-434` -**Severity:** MEDIUM -**Effort:** 15 minutes - -**Problem:** -```rust -let visible_row = list_row + state.scroll_offset; // ← No bounds check before indexing -``` - -**Risk:** Index out of bounds panic if clicking below visible list. - -**Fix:** Add bounds check before using `visible_row`. - ---- - -### 🟡 MEDIUM #11: Duplicate Ecosystem Filtering Logic - -**Status:** ❌ UNSOLVED -**Location:** `src/args.rs:94-110` + `src/ui/tui.rs:115-127` -**Severity:** MEDIUM -**Effort:** 20 minutes - -**Problem:** Same filtering logic in two places. - -**Impact:** Changes must be mirrored; risk of divergence. - -**Fix:** Extract to shared function or call `args.get_ecosystems()` instead of reimplementing. - -*(Also mentioned in CRITICAL #2)* - ---- - -### 🟡 MEDIUM #12: No Validation of Ecosystem Configuration Files - -**Status:** ❌ UNSOLVED -**Location:** `src/config/ecosystem.rs:95-107` -**Severity:** MEDIUM -**Effort:** 15 minutes - -**Problem:** - -If all ecosystem JSON files are corrupt, app runs with 0 ecosystems (no error to user). - -```rust -pub fn load_ecosystems() -> anyhow::Result> { - let mut ecosystems = Vec::new(); - for entry in glob::glob("ecosystems/*.json")? { - match entry { - Ok(path) => match load_ecosystem_from_path(&path) { - Ok(eco) => ecosystems.push(eco), - Err(e) => eprintln!("Warning: failed to load {:?}: {}", path, e), // ← Just warns! - }, - } - } - Ok(ecosystems) // ← Could be empty! -} -``` - -**Fix:** Return error if no valid ecosystems loaded: -```rust -if ecosystems.is_empty() { - return Err(anyhow!("No valid ecosystems found in ecosystems/")); -} -``` - ---- - -### 🟡 MEDIUM #13: Size Loading State Is Unclear - -**Status:** ❌ UNSOLVED -**Location:** `src/config/ecosystem.rs:84` -**Severity:** MEDIUM -**Effort:** 30 minutes - -**Problem:** Folders show "…" during loading with no indication of progress. - -**Impact:** Users don't know if app is frozen or just calculating. - -**Fix:** Add `SizeState` enum instead of just `u64`: -```rust -pub enum SizeBytes { - Unknown, - Calculating, - Known(u64), -} -``` - ---- - -### 🟡 MEDIUM #14: No Recovery From Partial Deletion - -**Status:** ❌ UNSOLVED -**Location:** `src/deleter.rs:24-65` -**Severity:** MEDIUM -**Effort:** 60 minutes - -**Problem:** - -If deletion fails midway (e.g., permission denied on subdir), folder is partially deleted but app reports success. - -**Impact:** Folder partially deleted; next run might not find it; user doesn't know cleanup is incomplete. - -**Fix:** Use trash crate or atomic moves instead of `remove_dir_all()`. - ---- - -### 🟡 MEDIUM #15: Wrong Default for Confidence Enum - -**Status:** ❌ UNSOLVED -**Location:** `src/config/ecosystem.rs:8` -**Severity:** MEDIUM -**Effort:** 5 minutes - -**Problem:** -```rust -#[derive(Default)] -pub enum Confidence { - #[default] - Certain, // ← But should be Undetected! - ... -} -``` - -When `DiscoveredFolder::new()` is called directly (in tests), confidence defaults to Certain. But folders are only "Certain" if they matched unambiguously. - -**Fix:** Change default to `Undetected`. - ---- - -### 🟡 MEDIUM #16: Unclear Default Ecosystem Behavior - -**Status:** ❌ UNSOLVED -**Location:** `src/args.rs:107-109` -**Severity:** MEDIUM -**Effort:** 15 minutes - -**Problem:** - -Documentation claims "use local patterns when no --target", but code returns ALL ecosystems when neither `--all` nor `--target` specified. - -**Impact:** Ambiguous intended behavior; hard to maintain. - -**Fix:** Clarify docs or change implementation to match intended design. - ---- - ## Low-Priority Issues (Optional) ### 🟢 LOW #17-25: Nine Low-Priority Improvements diff --git a/src/args.rs b/src/args.rs index 0043066..416478f 100644 --- a/src/args.rs +++ b/src/args.rs @@ -90,7 +90,8 @@ impl Args { } } - /// Get ecosystems to scan + /// Get ecosystems to scan. + /// Returns all ecosystems when neither `--all` nor `--target` is specified. pub fn get_ecosystems(&self, all_ecosystems: &[Ecosystem]) -> Vec { if self.all { all_ecosystems.to_vec() diff --git a/src/config/ecosystem.rs b/src/config/ecosystem.rs index c93bf05..625379b 100644 --- a/src/config/ecosystem.rs +++ b/src/config/ecosystem.rs @@ -6,10 +6,10 @@ use std::path::{Path, PathBuf}; /// How certain we are that a discovered folder belongs to an ecosystem. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum Confidence { - #[default] Certain, Confirmed, Ambiguous, + #[default] Undetected, } @@ -103,6 +103,9 @@ pub fn load_ecosystems() -> anyhow::Result> { Err(e) => eprintln!("Warning: glob error: {}", e), } } + if ecosystems.is_empty() { + return Err(anyhow::anyhow!("No valid ecosystems found in ecosystems/")); + } Ok(ecosystems) } diff --git a/src/deleter.rs b/src/deleter.rs index f743a66..96253d1 100644 --- a/src/deleter.rs +++ b/src/deleter.rs @@ -39,7 +39,7 @@ pub fn delete_folders(folders: &[DiscoveredFolder], dry_run: bool) -> DeleteSumm summary.deleted_count += 1; summary.freed_bytes += folder.size_bytes; } else { - match std::fs::remove_dir_all(&folder.path) { + match trash::delete(&folder.path) { Ok(_) => { println!( "Deleted: {} ({} - {})", diff --git a/src/scanner/dir.rs b/src/scanner/dir.rs index 53b4ab8..3a76e49 100644 --- a/src/scanner/dir.rs +++ b/src/scanner/dir.rs @@ -88,9 +88,16 @@ pub fn scan_directory( walker = walker.max_depth(depth); } + let mut all_exclude_dirs: Vec = exclude_dirs.to_vec(); + for &skip in SKIP_DIRS { + if !all_exclude_dirs.contains(&skip.to_string()) { + all_exclude_dirs.push(skip.to_string()); + } + } + for entry in walker .into_iter() - .filter_entry(|e| !should_skip_entry(e, exclude_dirs, exclude_hidden)) + .filter_entry(|e| !should_skip_entry(e, &all_exclude_dirs, exclude_hidden)) { match entry { Ok(entry) => { @@ -140,7 +147,7 @@ pub fn scan_directory( folders .into_iter() - .filter(|f| !path_contains_skip_dir(&f.path)) + .filter(|f| !path_contains_skip_dir(&f.path, &all_exclude_dirs)) .collect() } @@ -156,28 +163,20 @@ fn should_skip_entry( return true; } - if exclude_dirs.iter().any(|d| name_str == *d) { - return true; - } - - SKIP_DIRS.contains(&name_str.as_ref()) + exclude_dirs.iter().any(|d| name_str == *d) } -fn path_contains_skip_dir(path: &Path) -> bool { +fn path_contains_skip_dir(path: &Path, exclude_dirs: &[String]) -> bool { path.components().any(|c| { if let std::path::Component::Normal(name) = c { - should_skip(name) + let name_str = name.to_string_lossy(); + exclude_dirs.iter().any(|d| name_str == *d) } else { false } }) } -fn should_skip(name: &std::ffi::OsStr) -> bool { - let name_str = name.to_string_lossy(); - SKIP_DIRS.iter().any(|s| name_str == *s) -} - #[cfg(unix)] fn get_unique_id(path: &Path) -> Option { use std::os::unix::fs::MetadataExt; diff --git a/src/scanner/size.rs b/src/scanner/size.rs index b5c3b07..4ddd600 100644 --- a/src/scanner/size.rs +++ b/src/scanner/size.rs @@ -11,7 +11,7 @@ pub fn calculate_size(path: &Path) -> std::io::Result { { if entry.file_type().is_file() { if let Ok(metadata) = entry.metadata() { - total_size += metadata.len(); + total_size = total_size.saturating_add(metadata.len()); } } } diff --git a/src/ui/tui.rs b/src/ui/tui.rs index 2257174..47d132a 100644 --- a/src/ui/tui.rs +++ b/src/ui/tui.rs @@ -107,6 +107,7 @@ fn spawn_scan_thread(args: &Args) -> Receiver { Ok(e) => e, Err(err) => { let _ = tx.send(ScanEvent::Error(err.to_string())); + eprintln!("Warning: scan thread failed to load ecosystems: {}", err); return; } }; @@ -126,7 +127,8 @@ fn spawn_scan_thread(args: &Args) -> Receiver { // Send each folder as it's "found" (scan_directory returns a batch, so we stream them) for folder in &folders { if tx.send(ScanEvent::FolderFound(folder.clone())).is_err() { - return; // receiver dropped (user quit) + eprintln!("Debug: scan thread exiting - receiver dropped (user quit)"); + return; } } @@ -142,6 +144,7 @@ fn spawn_scan_thread(args: &Args) -> Receiver { }) .is_err() { + eprintln!("Debug: scan thread exiting - receiver dropped (user quit)"); return; } } @@ -176,9 +179,7 @@ fn run_app(terminal: &mut Terminal>, args: Args) -> any let term_size = terminal.size().unwrap_or_default(); let term_width = term_size.width; terminal.draw(|frame| { - let area = frame.area(); - let viewport_height = compute_list_height(area.height, term_width) as usize; - render(frame, &mut state, term_width, viewport_height); + render(frame, &mut state, term_width); })?; // Poll for input with remaining tick budget @@ -234,12 +235,7 @@ fn build_layout(area: Rect, term_width: u16) -> (Rect, Rect, Rect) { // Rendering // --------------------------------------------------------------------------- -fn render( - frame: &mut ratatui::Frame, - state: &mut AppState, - term_width: u16, - _viewport_height: usize, -) { +fn render(frame: &mut ratatui::Frame, state: &mut AppState, term_width: u16) { let area = frame.area(); let (header_area, list_area, footer_area) = build_layout(area, term_width);