Implement a type for strings with pre-computed length

[pantsman0]

I have a type that is basically the EmbeddedString type from the doc example, and wanted to contribute it back for other users.

I know this is basically the same as the below, but seemed useful anyway:

 #[binread]
 #[derive(Debug)]
 struct EmbeddedString {
     #[br(temp)]
     _len: u8,
     #[br(parse_with = count(_len.into()))]
     string: Vec<u8>
 }

[kitlith]

hm. Isn't the FixedLengthString contributed in this PR equivalent to the following?

#[binrw]
#[br(import(count: usize))]
struct FixedLengthString(
    #[br(args { count })]
    // or equivilantly
    // #[br(count = count)]
    pub Vec<u8>
)

which... i'm not sure that really adds anything, aside from using args(count) over args { count: expr } or count = expr. Feel free to argue the case, though.

Aside, I don't believe there is anything preventing us from using the derive macro within binrw itself.

Something that might be neat, though, is an optional dependency on https://crates.io/crates/bstr and binread/binwrite implementations for it.

[csnover]

Thanks for your PR!

I agree with @kitlith’s analysis that this patch as written doesn’t appear to add much value as it’s the same thing as the pre-existing Vec<u8> impl and/or count helper. I guess the one difference is that the wrapper has stringy Display and Debug traits, which isn’t nothing, but it does reiterate the point from a recent discussion in chat about with/repr that a concrete type, and a serialiser for that type, aren’t the same thing (especially when it comes to primitives like String).

As kitlith also mentioned, this (and the existing NullString and NullWideString types) are essentially bstr-but-worse (because bstr includes most of the usual string manipulation methods too, not just a stringy Display implementation). So we should probably work toward providing a single wrapper type like that and, separately, having multiple serialisation options. In the medium-term (within the next release cycle), it’s plausible that these wrapper types disappear entirely in lieu of converters/serialisers that read directly into a Vec<u8> or String instead (#98) since I have already done preliminary work on transforming the API in that way.

The CI identifies some linting issues. Also, any tests of public interfaces need to go to the tests/strings.rs instead of having inline test module. I don’t think it is important to think much about these things now, except from the perspective of things that might be good to know for any future contribution which I look forward to seeing :-).

So, I would also opt not to land this PR, but again genuinely appreciate the effort.

[v1gnesh]

Something that might be neat, though, is an optional dependency on https://crates.io/crates/bstr and binread/binwrite implementations for it.

Yes please, I have a data source for which I don't need utf8 validation.

[csnover]

Closing this for the reasons mentioned earlier. Thank you again for your effort! I look forward to reviewing any future contributions.