bstr icon indicating copy to clipboard operation
bstr copied to clipboard

`OsStr` functions have generic names but fill a specific niche

Open epage opened this issue 3 years ago • 2 comments

The change for 1.0 mentioned in #40

ByteVec::into_os_string now returns Result<OsString, FromUtf8Error> instead of Result<OsString, Vec>.

Made me consider this.

In the docs, it says

  1. One could re-implement WTF-8 and re-encode file paths on Windows to WTF-8 by accessing their underlying 16-bit integer representation. Unfortunately, this isn’t zero cost (it introduces a second WTF-8 decoding step) and it’s not clear this is a good thing to do, since WTF-8 should ideally remain an internal implementation detail. ...

While this library may provide facilities for (1) in the future, currently, this library only provides facilities for (2) and (3).

(1) could be simplified, depending on what is done with OsStr, like https://github.com/rust-lang/rust/pull/95290

Should the existing functions have more specific names to open the door for the more general functions to be added later, if possible?

epage avatar Jul 11 '22 23:07 epage

Hmmm. What about a different idea. What if we tweak the wording of the existing conversion routines in bstr such that they could be implemented with the raw byte representation of OsStr, assuming it gets exposed? For example, ByteVec::from_os_string is currently documented as:

Create a new byte string from an owned OS string.

On Unix, this always succeeds and is zero cost. On non-Unix systems, this returns the original OS string if it is not valid UTF-8.

But we could maybe change this to:

Create a new byte string from an owned OS string.

On Unix, this always succeeds and is zero cost. On non-Unix systems, this may return the original OS string if it is not valid UTF-8 and the underlying bytes are otherwise not accessible.

I think this is ultimately the intent of these routines. And in particular, if std does end up exposing the OsStr byte representation in a platform independent way, then I think most of these routines become superfluous. I only added them precisely because OsStr's representation isn't exposed.

BurntSushi avatar Jul 12 '22 00:07 BurntSushi

cc @JoshTriplett

BurntSushi avatar Jul 12 '22 00:07 BurntSushi