Skip to content

Conversation

@ch3pjw
Copy link

@ch3pjw ch3pjw commented Dec 13, 2023

This is intended to be akin to the FromStr implementation for String found in std::string.

@ch3pjw
Copy link
Author

ch3pjw commented Jan 15, 2024

(Just updated because of merge conflicts)

@mcy
Copy link
Owner

mcy commented Jan 17, 2024

Hmm. I've been mulling this order. I am not a fan of how this not reflect the existing From impl, which is from &'static str since it does not require a copy... do you have a use-case in mind? The only thing it enables is str.parse::<Yarn>() which feels... not super useful?

@ch3pjw
Copy link
Author

ch3pjw commented Jan 17, 2024

Basically it enables some generic code that I'm working on that uses FromStr in its interface.

I have a generic interface to build a path structure that parses path segments which can be various types. The (simplified) interface looks like this:

impl<Seg> Path<Seg> {
    fn from_strs<S: AsRef<str>>(
        segs: impl IntoIterator<Item = S>,
    ) -> Result<Self, <Seg as FromStr>::Err>
    where
        Seg: FromStr
    {
        ...
    }
}

Originally I was using Strings for the segment type for a simple Path, but I wanted to use Yarn because it was more efficient for smaller string sizes, which are common in my paths. Yarn was pretty much a drop in replacement for String everywhere, but doesn't conform to this interface, so I thought adding the impl might be of use.

I don't think you'd be able to write a version of FromStr that doesn't use Yarn::copy, if my understanding of lifetimes is correct, because you can't access the lifetime of the given &str in the FromStr interface. (That is, I don't think FromStr could ever be a synonym for From<&'a Str>.) That is at least justification for the difference.

An alternative would be to re-write the interface to use a custom trait, which is not that big a deal. The situation just highlighted that Yarn was not so easy to drop in in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants