Conversation
|
r? @bluss (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
src/libstd/path.rs
Outdated
There was a problem hiding this comment.
Adding this variant is a breaking change
There was a problem hiding this comment.
Understood, how should this be addressed?
There was a problem hiding this comment.
Good question. The docs around Prefix imply that it's meant to be Windows-exclusive, so perhaps defining another type is the way to go? However, the only place where Prefix seems to be used is as part of Component, which is also a fully public enum and can't be extended with another kind of prefix.
It doesn't look like this part of the standard library was designed with this kind of extensibility in mind. Maybe in the next epoch the definition of Prefix could be changed (not sure if epochs allow these kinds of changes)?
Alternatively, you could define a std::os::redox::path::PathExt trait that's implemented by Path and allows access to the prefix. However, this does not allow Path::components to work like it should...
There was a problem hiding this comment.
not sure if epochs allow these kinds of changes
I've also thought of that, but I don't think it would work (though I'd love to be corrected!). How would that work? I guess you would somehow change the version of the Components enum and components() function being used depending on the epoch. But what if code using it then tried to pass the Components to code in the older epoch? Though perhaps an unlikely use case for this particular struct, that probably shouldn't error complaining that the types are different, and couldn't work because the code it's passed to won't know how to deal with the new variant.
kennytm
added
the
T-libs-api
|
Ping from triage @bluss! This PR needs your review. |
|
Ping from triage! This PR needs a review, can @bluss or someone else from @rust-lang/libs review this? |
TimNN
removed
the
S-waiting-on-review
alexcrichton
added
S-waiting-on-author
|
The libs team discussed this yesterday and our conclusion was that we indeed cannot extend a stable enum, but we'd be happy to accept a patch that has a redox-specific extension trait! |
|
Thanks @alexcrichton for the information. I believe more information about the problem I would like to solve is necessary, so I detailed it here, as well as potential solutions: #52331 |
|
Ok! Should this be closed while that's decided? |
|
Yes. I would expect to reopen with the chosen solution. |
9 participants
This adds Scheme to the path::Prefix enum and fixes some issues with path handling on Redox