tokio-rs / bytes

Utilities for working with bytes
MIT License
1.87k stars 278 forks source link

Add support for Nom #624

Closed nm17 closed 1 year ago

nm17 commented 1 year ago

In order for Nom to understand Bytes, we need to implement their needed traits. I've already added support for Nom 7.x on my fork and can create a PR for it, but it will work only on 7.x version of Nom. Their next major release breaks everything. If you are okay with only Nom 7.x support, I can submit a PR. If you want, I can figure out a way to support both versions.

You can also assign me on this issue if you want to, I'll try my best to figure this out.

Darksonn commented 1 year ago

If you are okay with only Nom 7.x support

Unfortunately, we don't want to commit to something like this. Tokio 1.x.y depends on bytes 1.x.y, so a breaking change to bytes would necessitate a breaking change to Tokio, and we don't want breaking changes in Tokio.

nm17 commented 1 year ago

@Darksonn Is it a breaking change? All that's needed is to add a new module to bytes (named something like nom_7 like how serde support is done) that will contain all the needed implementations. I think we can support other versions of Nom with a dependency entry like this one:

nom-7 = { package = "nom", version = "7", optional = true, ... }

The only reason why I'm suggesting to add this directly is because there is no way to implement traits for stuff from third-party crates without wrapping types and implementing everything for them.

Darksonn commented 1 year ago

It would be a breaking change to upgrade nom in case a version 8 is released. You're correct that we could depend on several versions, but I really would rather not have an ever increasing list of nom dependencies.

I know it's more convenient for you if we add this, but we have to support this forever, and I don't want to do that.

seanmonstar commented 1 year ago

In your own project, you could create a struct NomBytes(pub Bytes), and implement the traits you need.