libs-team
libs-team copied to clipboard
impl PathBuf::add_extension and Path::with_extra_extension
Proposal
Problem statement
Sometimes, the program can generate files with its own suffix (extension) to identify specific purpose files.
For example, in my tools there is a dry-run mode for formatting files (link):
let mut extension = doc.filepath.extension().unwrap_or_default().to_os_string();
extension.push(".formatted");
let copied = doc.filepath.with_extension(extension);
doc.save(Some(&copied))
If we have an add_extension
method here to append extra extension, the code can be simplified as:
let copied = doc.filepath.with_extra_extension(".formatted");
doc.save(Some(&copied))
This situation can be applied for .bak
or other cases.
PathBuf::add_extension(&mut self, extension: impl AsRef<OsStr>)
is an additional method to modify the PathBuf in place without construct a brand-new instance.
Motivating examples or use cases
Included above.
Solution sketch
It's more expressive with a patch; see https://github.com/rust-lang/rust/pull/123600:
pub fn add_extension<S: AsRef<OsStr>>(&mut self, extension: S) -> bool {
self._add_extension(extension.as_ref())
}
fn _add_extension(&mut self, extension: &OsStr) -> bool {
let file_name = match self.file_name() {
None => return false,
Some(f) => f.as_encoded_bytes(),
};
let new = extension.as_encoded_bytes();
if !new.is_empty() {
// truncate until right after the file name
// this is necessary for trimming the trailing slash
let end_file_name = file_name[file_name.len()..].as_ptr().addr();
let start = self.inner.as_encoded_bytes().as_ptr().addr();
let v = self.as_mut_vec();
v.truncate(end_file_name.wrapping_sub(start));
// append the new extension
v.reserve_exact(new.len() + 1);
v.push(b'.');
v.extend_from_slice(new);
}
true
}
Alternatives
Not applicable. This is a trivial case somewhat.
Links and related work
https://github.com/rust-lang/rust/pull/123600:
What happens now?
This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.
Possible responses
The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):
- We think this problem seems worth solving, and the standard library might be the right place to solve it.
- We think that this probably doesn't belong in the standard library.
Second, if there's a concrete solution:
- We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
- We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
+1 but your "dry-run" code expected copied
being a PathBuf
, yet your proposed add_extension()
returns a bool. I suppose the simplified code should be:
let mut copied = doc.filepath.clone();
copied.add_extension("formatted"); // should actually handle a `false` result
doc.save(Some(&copied));
Alternatively (additionally?) maybe you actually want a &Path -> PathBuf
method
impl Path {
fn with_extra_extension(&self, extension: impl AsRef<OsStr>) -> PathBuf { ... }
}
let copied = doc.filepath.with_extra_extension("formatted");
doc.save(Some(&copied));
@kennytm Thanks for your input. Correct that my sample code has a bug.
I agree that we can additionally add a Path:: with_extra_extension
method, since if the case is it can modify the PathBuf
in place, the user can avoid construct a new instance.
Let me update in the PR and issue description.
@joboet is there a specific timeline that lib-team would pick up this issue? Or how can I add this issue in the schedule?
Just be patient. There's quite a backlog but they'll get to it eventually.