prost icon indicating copy to clipboard operation
prost copied to clipboard

Support for String<Bytes> or similar

Open vorner opened this issue 4 years ago • 13 comments

Hello

Thanks for #31, I can't wait for a release 😇.

Anyway, it would be nice if strings could get similar support for referencing the Bytes buffer too.

I don't have a preferred/thought through solution, only some ideas how that could work:

  • Picking a crate that does this kind of wrapping of Bytes into something derefing to &str (there were some mentioned in #31, I don't know how well maintained they are), maybe make the dependency optional. Then the same option would change String -> String<Bytes> as the one that currently changes Vec<u8> -> Bytes.
  • Allow the user to configure type replacements on fields, eg. saying that field_type_replace("String", "Whatever", &["paths"]). That type would have to support Message already in some way, but that would be up to the user. That could probably be used to replace „nested“ fields encoded as bytes by the actual thing inside.

But anything that works would be nice :-).

Thank you

vorner avatar Nov 15 '20 21:11 vorner

Thinking about it more, that field_type_replace approach would be more flexible, as it would also allow using things like smallstring or smallvec at places, to optimize things.

:question: If I tried writing the support for prost-build, would there be interest in accepting it (after figuring out proper naming, what to do with generics in the Vec -> SmallVec case, etc), or would that be considered out of scope for the crate?

vorner avatar Jan 15 '21 08:01 vorner

Yes I'd like to support this, in fact a recent refactor in https://github.com/danburkert/prost/commit/79f0dfd8ed703fcce27f053c3b29c360c7ead5cb was laying the groundwork. I don't think the set of types you will be able to substitue will be able to be 'open', because prost-derive is going to have to know how to encode/decode them. So I'd like to have an API which allows you to select from an enum, with String/Bytes/Secret variants.

danburkert avatar Jan 15 '21 19:01 danburkert

secret from #369

danburkert avatar Jan 15 '21 19:01 danburkert

About being open… I was wondering if there would be a trait these things need to satisfy (it probably isn't prost::Message, is it?). For Vec for repeated fields, it could possibly be something like Extend, what to do about string or bytes fields ‒ I'm not sure there's an existing trait describing that one.

Right now I'm looking for a good String(Bytes) thing on crates.io and deciding if there is one or if I want to roll my own (it would be nice if such string thing could have auxiliary functions like split or lines that return these owned String(Bytes) instead of &str, for example). But I would probably not expect that to be directly supported by prost and it would still be nice to be able to somehow plug it in ‒ be it by having the right trait, or duck-typing (it generates code, having a same-named method would be enough)

(Yes, if the string field is represented as Bytes, I can wrap it into such type manually, so it's not a huge deal, but it would still be convenient to be supported out of the box, without manual post-processing).

vorner avatar Jan 16 '21 10:01 vorner

If anyone is interested in longer and more verbose description of what I have in mind: https://vorner.github.io/2021/01/31/saving-some-allocations.html

If you'd be interested in doing it with the traits as I mentioned above, I get my hands dirty and see if I can come up with a reasonable PR for that.

vorner avatar Jan 31 '21 10:01 vorner

I want this feature too. @vorner are you still working on it? I'd also like field_encoding_replace that instead of Message implemented on type uses a custom module. This would be similar to #[serde(with = "some::module")]. I noticed integers are already implemented as modules.

Kixunil avatar Dec 17 '21 22:12 Kixunil

I randomly noticed there's a substitutions branch - it seems to implement this. Maybe just resolve the conflict and merge?

Kixunil avatar Dec 17 '21 22:12 Kixunil

are you still working on it?

Not actively. I have that bytes-utils crate around, and I'm intent on keeping it alive, but otherwise don't have any actual plans or any work in progress around it. Besides, I currently don't have the need for that feature any more (but I still think it would be a neat thing).

vorner avatar Dec 18 '21 09:12 vorner

OK, will try look into making it myself.

Kixunil avatar Dec 18 '21 10:12 Kixunil

Is anyone currently working on this?

I have some use case for the generic version for supporting url::Url fields (through in difference to the other examples here that would be a fail-able parsing :thinking: ).

Depending on how much work it is I might(1) be able to contribute it.

(1): I will most likely know weather or not I could contribute, and how much time I could spend on it later this week.

rustonaut avatar May 10 '22 12:05 rustonaut

No one is currently working on this at the moment.

LucioFranco avatar May 23 '22 18:05 LucioFranco

thanks :+1:

------- Original Message ------- On Monday, May 23rd, 2022 at 8:50 PM, Lucio Franco @.***> wrote:

No one is currently working on this at the moment.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

rustonaut avatar May 23 '22 18:05 rustonaut

@rustonaut I did spend some time on it but it's more complex than I expected, so couldn't finish it.

Kixunil avatar Jun 17 '22 09:06 Kixunil