`into_join` for `PathBuf`

language design
1

It bugs me that calling join on PathBuf allocates a new PathBuf instead of extending the existing one. Example:

returns_path_buf().join(path)

But I also see why people don't want to use push in such cases:

let mut path_buf = returns_path_buf();
path_buf.push(path);
path_buf

So my proposal is to add the following method to PathBuf:

impl PathBuf {
    #[inline]
    pub fn into_join<P: AsRef<Path>>(mut self, path: P) -> Self {
        self.push(path);
        self
    }
}

So the example above becomes:

returns_path_buf().into_join(path)

Of course, this can be chained:

returns_path_buf().into_join(path1).into_join(path2)

I would be happy to implement it myself and document it :slight_smile:

4 Likes
2

Yes please.

You should open an API Change Proposal.

2 Likes
3

Consider using the name append or concat.

9 Likes
4

Note that this potentially reallocates twice, once for path1 and once for path2, whereas one could define an interface that can avoid it by taking multiple inputs and counting the total length:

returns_path_buf().into_join([path1, path2])

This could be useful because path manipulation often looks like joining {base_dir}/{subdir}/{filename} (e.g. in Cargo, {target_dir}/{profile_name}/{binary_target_name}).

1 Like
5

To mirror @pitaj 's comment, a natural name would be extend.

Would this accept an IntoIterator?

6

Unfortunately that could cause name clashes with the Extend trait (or more accurately, the extend() method within it), if a decision were ever made to implement Extend for PathBuf.

7

Extend is already implemented for PathBuf

1 Like
8

I do think it'd be nice to improve upon the ergonomics of extend. Here's a few ideas I've previously sketched out (playground link). Note that the names of things are purely placeholders.

use std::ffi::OsStr;
use std::path::{Path, PathBuf};

fn fn_that_returns_a_pathbuf() -> PathBuf {
    PathBuf::from("root")
}

fn main() {
    let mut p = fn_that_returns_a_pathbuf().with_all((OsStr::new("a"), "b", Path::new("c")));
    println!("{}", p.display());
    p.extend_all((Path::new("d"), "e"));
    println!("{}", p.display());

    let p2 = (fn_that_returns_a_pathbuf(), "foo", OsStr::new("bar"), Path::new("baz")).combine_paths();
    println!("{}", p2.display());
}

trait ExtendAllPath {
    fn extend_all(p: &mut PathBuf, other: Self);
}
impl<T1: AsRef<Path>> ExtendAllPath for (T1,) {
    fn extend_all(p: &mut PathBuf, other: Self) {
        p.push(other.0);
    }
}
impl<T1: AsRef<Path>, T2: AsRef<Path>> ExtendAllPath for (T1,T2) {
    fn extend_all(p: &mut PathBuf, other: Self) {
        p.push(other.0);
        p.push(other.1);
    }
}
impl<T1: AsRef<Path>, T2: AsRef<Path>, T3: AsRef<Path>> ExtendAllPath for (T1,T2,T3) {
    fn extend_all(p: &mut PathBuf, other: Self) {
        p.push(other.0);
        p.push(other.1);
        p.push(other.2)
    }
}

// We're not in std so we need an extra trait to add functions to PathBuf.
trait PathBufExtendAny {
    fn extend_all<T: ExtendAllPath>(&mut self, other: T) -> &mut Self;
    fn with_all<T: ExtendAllPath>(self, other: T) -> Self;
}
impl PathBufExtendAny for PathBuf {
    fn extend_all<T: ExtendAllPath>(&mut self, other: T) -> &mut Self {
        T::extend_all(self, other);
        self
    }
    fn with_all<T: ExtendAllPath>(mut self, other: T) -> Self {
        T::extend_all(&mut self, other);
        self
    }
}

trait CombinePathTuple {
    fn combine_paths(self) -> PathBuf;
}
impl<T1: AsRef<Path>> CombinePathTuple for (PathBuf, T1) {
    fn combine_paths(mut self) -> PathBuf {
        self.0.push(self.1);
        self.0
    }
}
impl<T1: AsRef<Path>, T2: AsRef<Path>> CombinePathTuple for (PathBuf, T1, T2) {
    fn combine_paths(mut self) -> PathBuf {
        self.0.push(self.1);
        self.0.push(self.2);
        self.0
    }
}
impl<T1: AsRef<Path>, T2: AsRef<Path>, T3: AsRef<Path>> CombinePathTuple for (PathBuf, T1, T2, T3) {
    fn combine_paths(mut self) -> PathBuf {
        self.0.push(self.1);
        self.0.push(self.2);
        self.0.push(self.3);
        self.0
    }
}
9

I prefer into_join because it makes it obvious that the only difference to join is that self is moved.

All append methods in std take &mut self.

concat can be misleading. It implies something like string concatenation. But joining with an absolute path replaces the root path completely.

That interface would be the equivalent of extend but taking ownership. I am proposing an equivalent of push but taking ownership. We can add both methods, but I think that the second one is more controversial. Something like this?

pub fn into_extend<S, T>(mut self, paths: S) -> Self
where
    S: AsRef<[T]>,
    T: AsRef<Path>,
{
    let paths = paths.as_ref();
    self.reserve(
        paths
            .iter()
            .map(|path| path.as_ref().as_os_str().len())
            .sum::<usize>()
            + paths.len() * const { MAIN_SEPARATOR.len_utf8() },
    );
    self.extend(paths);
    self
}

BTW, the current implementation of extend on PathBuf doesn't reserve capacity. It only calls push for every path because it takes an iterator, not a slice.

It is not always worth it to reserve capacity for all additional paths because the resulted path could be shorter in the case of something like .. or an absolute path which replaces the root.

Anyway, let's please open a new discussion for this if you want to have it. I would like to keep this thread about the equivalent of push for only one path.

10

Proposal on Github:

11

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.