Conversation
0d0f6ba to
6f25c6f
Compare
Owner
Author
|
Unfortunately the supertrait sealing makes the implementers not visible in the documentation, which I think is crucial to have here. We can explore other options, although at the same time: are these suggestions showing up even when the trait isn't imported? There should rarely be a need for anybody to import the trait. |
Owner
Author
|
Hm, maybe we could still use super trait and have the methods on it, but instead of having a generic |
Owner
Author
14638bb to
44eb45b
Compare
* `impl AsSymbolName for &[u16]` hides unconditional allocation (user could pass a `&str` or `&[u8]` or `&CStr` instead all of which should be more straightforward. * `impl AsFilename for &[T]` have awkward platform implications. With `T = u16` we require that buffer is valid UTF-16, for `T = u8` we require that it is UTF-8 but only on Windows. I'm not super comfortable with having these types in the cross-platform API surface currently. We can explore adding something as a follow-up, but for now no-std users will have to live with passing in a `&str`. * Similarly for `impl AsFilename for CString` family. * There are a couple of no-std capable tests that sufficiently test the functionality of no-std behaviour. For the rest it is fine to require that libstd is available. So this removes most of the duplicate no-std tests.
44eb45b to
e1416f7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

cc @AlexanderSchuetz97 double-check that this works for you.
I ended up nuking a number of
AsFilenameandAsSymbolNameimpls still and made the remaining ones terser. We can easily add them back later on in a backwards-compatible manner if it turns out that the current impls don't serve some specific need for some reason or another.