refactor: use and simplify is_dir#776
Conversation
There was a problem hiding this comment.
This refactoring successfully consolidates the is_dir logic into a single, reusable function in src/spec.rs and simplifies the implementation. The change from manual character iteration to ends_with(['/', '\\']) is cleaner and more idiomatic. All usages have been properly updated to use the centralized function.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
There was a problem hiding this comment.
Code Review
This pull request refactors the is_dir logic by centralizing it into a utility function and simplifying the implementation using ends_with. It also cleans up the codebase by removing an unused dead_code attribute. The review feedback suggests a performance optimization for the is_dir function in src/spec.rs, recommending a byte-level check to avoid unnecessary UTF-8 decoding.
| .chars() | ||
| .next_back() | ||
| .is_some_and(|c| c == '/' || c == '\\') | ||
| filename.ends_with(['/', '\\']) |
There was a problem hiding this comment.
While ends_with is idiomatic, for a high-performance crate like zip, we can avoid UTF-8 decoding of the last character by performing a byte-level check. Since / and \ are ASCII, they are represented as single bytes in UTF-8 and cannot appear as part of a multi-byte sequence. This optimization is safe and more efficient for a core utility function.
| filename.ends_with(['/', '\\']) | |
| matches!(filename.as_bytes().last(), Some(b'/') | Some(b'\\')) |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: n4n5 <git@n4n5.dev>
Uh oh!
There was an error while loading. Please reload this page.