Add path manipulation utils #47
Conversation
sosthene-nitrokey
requested review from
nickray,
daringer and
robin-nitrokey
src/path.rs
Outdated
| pub fn file_name(&self) -> Option<&Path> { | ||
| let this = self.as_str_ref_with_trailing_nul(); | ||
| match this.rsplit_once('/') { | ||
| Some((_, path)) if path == "\x00" => None, |
There was a problem hiding this comment.
Indeed better.
src/path.rs
Outdated
| let this = self.as_str_ref_with_trailing_nul(); | ||
| match this.rsplit_once('/') { | ||
| Some((_, path)) if path == "\x00" => None, | ||
| None if this != "\x00" => Some(self), |
There was a problem hiding this comment.
nit: Check self.inner.is_empty() at the start of the method instead? Then this could be a map instead. Maybe easier to read.
There was a problem hiding this comment.
How would that allow getting rid of the Some((_, "\x00")) case? This case handles paths that end with a trailing slash but are not empty. We could return file.extension for path!("/some/path/file.extension/").file_name() but that would mean using either a PathBuf or &str as the return type.
I generally don't like using those, but it might be the best solution. It would match the behaviour of std.
There was a problem hiding this comment.
I curse C and its trailing 0. It's so unpractical to require copying and not having any ability to slice.
There was a problem hiding this comment.
It would not get rid of it, but it would get rid of the None case and an if in a map or a filter plus a map could be easier to read than the match. But that’s just a feeling. This version is fine too of course.
sosthene-nitrokey
force-pushed
the
path-utils
branch
from
ddb8415
to
e6c46e7
Compare
|
There had been a couple of addition since the first approval. |
No description provided.