feat: add --since flag for time-based file filtering (#1) #1687
Conversation
Co-authored-by: Copilot
Changes SummaryThis PR implements a new Type: feature Components Affected: CLI Options Parser, File Filtering System, Help Documentation, Test Suite Files Changed
Architecture Impact
Risk Areas: Timestamp comparison logic using max(modified_time, created_time) - cross-platform edge case with files that have no timestamps or only partial timestamp support, SystemTime::checked_sub() subtraction could fail on overflow; current code returns true (includes file) which is safe but conservative, chrono::NaiveDateTime conversion to SystemTime uses UNIX_EPOCH arithmetic; potential precision loss depending on platform's time representation, Integration tests depend on filesystem's ability to modify file times via filetime crate; behavior may vary across platforms (noted in test comments), Performance impact of checking every file's timestamps (two File API calls per file) during filtering Suggestions
Full review in progress... | Powered by diffray |
| /// Convert a `NaiveDateTime` to `SystemTime` for comparison | ||
| fn naive_datetime_to_systemtime(dt: Option<chrono::NaiveDateTime>) -> Option<std::time::SystemTime> { | ||
| dt.and_then(|dt| { | ||
| dt.and_utc().timestamp().try_into().ok().and_then(|secs: u64| { | ||
| std::time::SystemTime::UNIX_EPOCH.checked_add(std::time::Duration::from_secs(secs)) | ||
| }) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM - Private helper lacks edge case documentation
Agent: rust
Category: docs
Description:
The naive_datetime_to_systemtime() helper uses try_into() from i64 to u64 without documenting that negative timestamps (pre-1970) will return None. The is_recent_file() method has partial documentation but doesn't fully explain this edge case behavior.
Suggestion:
Add a brief doc comment to naive_datetime_to_systemtime() explaining that negative Unix timestamps are treated as unavailable (returns None).
Why this matters: Explicit contracts prevent undefined behavior and misuse.
Confidence: 65%
Rule: rs_document_safety_panics_and_errors
Review ID: 0b482805-d6db-4856-9263-a5b31a9b9b5a
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
| fn test_since_filter_with_short_window() { | ||
| let temp_dir = TempDir::new().expect("Failed to create temp dir"); | ||
|
|
||
| create_file_with_mtime(&temp_dir, "test.txt", 60); | ||
|
|
||
| std::thread::sleep(Duration::from_millis(150)); | ||
|
|
||
| let eza = get_eza_binary(); | ||
|
|
||
| let output = Command::new(&eza) | ||
| .arg("--since") | ||
| .arg("50ms") | ||
| .arg(temp_dir.path()) | ||
| .output() | ||
| .expect("Failed to execute eza"); | ||
|
|
||
| let stdout = String::from_utf8_lossy(&output.stdout); | ||
| assert!(!stdout.contains("test.txt"), "Should not show with very short window after delay"); | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM - Test name 'test_since_filter_with_short_window' is ambiguous
Agent: testing
Category: quality
Description:
Test name doesn't clarify the expected outcome (file exclusion). The test creates a 60-second-old file and uses a 50ms window, expecting the file to NOT be shown.
Suggestion:
Rename to 'test_since_filter_excludes_files_older_than_window' to clarify the boundary and expected behavior.
Why this matters: Descriptive test names serve as documentation and help debug failures.
Confidence: 65%
Rule: test_generic_test_names_rust
Review ID: 0b482805-d6db-4856-9263-a5b31a9b9b5a
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
| fn test_since_filter_invalid_duration() { | ||
| let temp_dir = TempDir::new().expect("Failed to create temp dir"); | ||
| create_file_with_mtime(&temp_dir, "test.txt", 60); | ||
|
|
||
| let eza = get_eza_binary(); | ||
|
|
||
| let output = Command::new(&eza) | ||
| .arg("--since") | ||
| .arg("invalid") | ||
| .arg(temp_dir.path()) | ||
| .output() | ||
| .expect("Failed to execute eza"); | ||
|
|
||
| assert!(!output.status.success(), "Should fail with invalid duration"); | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM - Test name doesn't describe expected behavior (failure)
Agent: testing
Category: quality
Description:
Test name 'test_since_filter_invalid_duration' describes the input but doesn't indicate the expected outcome (command should fail).
Suggestion:
Rename to 'test_since_filter_rejects_invalid_duration' to clearly indicate the expected failure behavior.
Why this matters: Descriptive test names serve as documentation and help debug failures.
Confidence: 65%
Rule: test_generic_test_names_rust
Review ID: 0b482805-d6db-4856-9263-a5b31a9b9b5a
Rate it 👍 or 👎 to improve future reviews | Powered by diffray
Review Summary
Validated 8 issues: 4 kept (documentation and test naming concerns), 4 filtered (minor style preferences and rule misapplication) Issues Found: 4💬 See 3 individual line comment(s) for details. 📊 2 unique issue type(s) across 4 location(s) 📋 Full issue list (click to expand)🟡 MEDIUM - Private helper lacks edge case documentation (2 occurrences)Agent: rust Category: docs Why this matters: Explicit contracts prevent undefined behavior and misuse. 📍 View all locations
Rule: 🟡 MEDIUM - Test name 'test_since_filter_with_short_window' is ambiguous (2 occurrences)Agent: testing Category: quality Why this matters: Descriptive test names serve as documentation and help debug failures. 📍 View all locations
Rule: ℹ️ 1 issue(s) outside PR diff (click to expand)
🟡 MEDIUM - Public FileFilter::deduce() lacks error documentationAgent: rust Category: docs Why this matters: Explicit contracts prevent undefined behavior and misuse. File: Description: The public deduce() method can return OptionsError::BadArgument from deduce_since_duration() but this error condition is not documented. Suggestion: Add '# Errors' section documenting that OptionsError::BadArgument can be returned if the --since duration string cannot be parsed. Confidence: 65% Rule: Review ID: |
2 participants
Co-authored-by: Copilot
Description
Adds
--since=DURATIONflag to filter files by recency. Files are shown if either theirmodified_timeorcreated_time(whichever is more recent) falls within the specified duration. Files without timestamps are excluded.Implementation:
humantimedependency for duration parsingsince_duration: Option<Duration>field toFileFilterfilter_child_files()andfilter_argument_files()--ignore-glob,--only-dirs, etc.)Filtering Logic:
The filter uses the maximum of created and modified timestamps, ensuring files appear if EITHER:
This provides intuitive behavior for users looking for recent activity.
Duration formats:
10s,5m,2h,7d,1w5 seconds,10 minutes,2 hoursExample usage:
Files modified:
Cargo.toml: Addedhumantimeand test dependencies (tempfile,filetime)src/options/flags.rs: Added SINCE flagsrc/options/filter.rs: Duration parsing logicsrc/fs/filter.rs: Timestamp filtering with helper function (using idiomatic let...else pattern and max() for timestamp comparison with comprehensive explanatory comments)src/options/help.rs,man/eza.1.md: DocumentationCHANGELOG.md: Added feature entrytests/since_filter.rs: 7 integration testsHow Has This Been Tested?
Original prompt
FEATURE: Implement --since as a functional filter (feature/since-filter)
Motivation
Users often need to quickly identify files that have been recently modified or created, especially in large directories or when using recursive views. While sorting by time is available, it still requires the user to scan through all files. A functional
--sincefilter allows users to restrict the output to only those files that fall within a specific time window, improving productivity and clarity.This is particularly useful for agentic workflows and automated monitoring where visualizing the creation of files as they appear is useful. For example, running
watch eza --tree --since 1mprovides a live, focused view of project activity without the noise of older files.Use Cases
watch eza --since 30sto see which files are being touched by a build process or a test suite in real-time.eza --since 1h /var/log.Goal
--since=<DURATION>filter that only displays files that were created or modified within the given duration (relative durations interpreted as now - duration).Scope & Constraints
--since=<DURATION>and useshumantimefor parsing (supporting e.g.5s,30m,2h,1d,1w, and ISO durations likePT5S).Branching & Workflow
feature/since-filter.High-level Plan (phases)
Phase 1 — Parsing & Option Plumbing
SINCEflag insrc/options/flags.rsand include it inALL_ARGS.Options::since_durationreturns Optionstd::time::Duration and useshumantime::parse_duration(already present; verify).since_durationinto a new field onFileFilter(e.g.,pub since_duration: Option<std::time::Duration>).Phase 2 — Filtering Logic
src/fs/filter.rs:filter_child_filesandfilter_argument_filesto apply thesince_durationpredicate.File, compute a recency timestamp as the maximum of the available timestamps (modified_time and created_time). If neither timestamp is available, treat the file conservatively as not recent (i.e., exclude it when--sinceis used).--sincefilter is an additional filter that reduces the file set.Phase 3 — Output Clean-up
src/output/file_name.rs(delete code that appended " *") and removesince_durationfield from output Options since it is no longer needed in output formatting.Phase 4 — Tests & Validation
Unit tests:
src/options/file_name.rsshould be extended if needed).Filetime values (if possible) and verify the expected behavior of the predicate.Integration tests:
tests/since_filter.rswhich usestempfile+filetimeto create test files with deterministic timestamps (old vs. recent) and runs the binary with--sinceasserting only the recent files are present.--since 1dfor a file with timestamp 30 minutes ago).Cross-platform:
Phase 5 — Docs & Man Page
src/options/help.rsto describe that--sincefilters files by recency (examples included).man/eza.1.mdshowingeza --since 24handeza --since 30m --tree.CHANGELOG.mdorCHANGELOGdescribing the feature change.Phase 6 — Acceptance & Release
cargo testpasses (unit & integratio...