Allow size attribute with depth option#137
Merged
Merged
Conversation
Previously, using the size attribute together with the depth option would throw an error. This restriction existed because depth limiting prevents full tree traversal, making recursive size calculations incomplete. This change removes the restriction and instead: - Returns undefined for size when a directory's children are not traversed due to depth limiting - Propagates undefined sizes up the tree (if any child has undefined size, the parent's size is also undefined) - Continues to calculate accurate sizes for files and fully-traversed directories This allows users to get partial size information when using depth limits, with undefined clearly indicating incomplete data. Fixes #136
- Updated README to explain that size can now be used with depth - Added dedicated section "Using size with depth" with clear examples - Explained that depth-limited directories get size: undefined - Updated CLI help text for both --depth and --attributes options - Replaced "prohibited" language with helpful guidance The documentation now clearly explains: - Files always have accurate sizes - Directories at depth limit have size: undefined - Parent directories propagate undefined when children are incomplete
Test improvements: - Made assertions mandatory instead of conditional - Added explicit check that test data contains both files and directories - Added assertion for root directory size propagation - Added helpful error messages for assertion failures Documentation improvements: - Changed code block from json to js (undefined is not valid JSON) - Added note explaining JSON.stringify() behavior with undefined - Clarified that undefined properties are omitted in JSON output These changes address code review feedback to make the test more robust and the documentation more accurate about runtime behavior.
Added 4 new test cases to thoroughly validate edge cases: 1. Empty directory with depth limit - Verifies empty directories at depth limit get size: undefined - Tests depth: 0 on some_dir_2 (empty directory) 2. Directory with only files at depth limit - Tests another_dir (contains only files) at depth: 0 - Verifies size: undefined when children not traversed 3. Multiple depth levels (depth: 2) - Tests nested structure with 3 levels (test_data/some_dir/another_dir) - Verifies size propagation through multiple levels - Confirms depth-limited nested dirs get size: undefined - Confirms parent propagates undefined upward 4. Backward compatibility - Tests size attribute WITHOUT depth option - Verifies all directories and files get numeric sizes - Confirms recursive size calculation works as before - Critical regression test for existing functionality Test Results: - All 4 new tests pass ✓ - Original enhanced test still passes ✓ - Total: 21 passing tests - No regressions introduced These tests provide comprehensive coverage of: - Edge cases (empty dirs, nested structures) - Size propagation logic - Backward compatibility - Multiple depth levels
Clarity improvements: - Changed "reads so many nested dirs as specified" to clearer "Limits directory traversal to specified depth" - Consistent terminology: "directories at depth limit" everywhere - More concise phrasing throughout Consistency fixes: - README options section now matches CLI help text exactly - Changed "may have" to "have" (behavior is deterministic) - Unified wording: "Combined with size" / "With depth" Conciseness improvements: - Removed redundant note that repeated earlier explanation - Shortened CLI help text while maintaining clarity - Simplified JSON serialization note All tests still pass (21/21). Documentation is now clearer, more consistent, and easier to understand at a glance.
|
This looks great! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, using the size attribute together with the depth option
would throw an error. This restriction existed because depth limiting
prevents full tree traversal, making recursive size calculations
incomplete.
This change removes the restriction and instead:
traversed due to depth limiting
size, the parent's size is also undefined)
directories
This allows users to get partial size information when using depth
limits, with undefined clearly indicating incomplete data.
Fixes #136