refactor(dirname): implement pure string manipulation per POSIX #8936
Conversation
Add test_missing_operand to cover error path when dirname called with no arguments (lines 80-82). Add test_multiple_paths_comprehensive to cover loop iteration with mixed edge cases including trailing dot paths, empty strings, and various path types (line 84).
Replace Path::parent() approach with byte-level string operations to avoid path normalization. This ensures dirname behaves as a pure string manipulation tool per POSIX/GNU specifications, correctly handling patterns like foo//., foo/./bar, and preserving /. components in middle of paths. Add comprehensive test coverage for edge cases including multiple slash variations, dot-slash patterns, and component preservation. Addresses issue uutils#8910 with complete POSIX-compliant implementation.
CodSpeed Performance ReportMerging this PR will improve performance by 4.08%Comparing Summary
Performance Changes
Footnotes
|
|
GNU testsuite comparison: |
|
@naoNao89 please don't start new pr when others started. Contribute to the other instead :) |
|
GNU testsuite comparison: |
naoNao89
force-pushed
the
test/dirname-additional-coverage
branch
from
63fe1ff to
dc5e0eb
Compare
|
why draft ? :) |
naoNao89
force-pushed
the
test/dirname-additional-coverage
branch
from
dc5e0eb to
2c72ad3
Compare
|
GNU testsuite comparison: |
|
@naoNao89 what's the status on this ? It has conflicts |
|
resolved |
|
GNU testsuite comparison: |
|
Caught an |
tests/by-util/test_dirname.rs
Outdated
| #[test] | ||
| fn test_missing_operand() { | ||
| // Test calling dirname with no arguments - should fail | ||
| // This covers the error path at line 80-82 in dirname.rs |
There was a problem hiding this comment.
| // This covers the error path at line 80-82 in dirname.rs |
lines will change
- Remove unnecessary comments from test_dirname.rs - Change dirname_string_manipulation() return type to Cow<'_, [u8]> - Add explicit lifetime annotation to resolve compiler warning - Optimize memory allocation with copy-on-write pattern
|
follow up #10294 |
3 participants
Fixes #8924 by rewriting dirname as pure byte-level string operations, avoiding Path::parent() normalization.
Handles all edge cases: foo//., foo///., foo/./, etc. Implementation uses explicit step-by-step logic without unsafe code, making it maintainable and platform-safe.
Added comprehensive tests for slash variations and dot-slash patterns.
Alternative to #8927 - this approach prioritizes code clarity and safety over conciseness. Both fix the same issues, but this avoids unsafe assumptions about OsStr encoding.