Skip to content

Add Encoding::label() #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

talklittle
Copy link

Motivation: Get a standard representation of an encoding that can be used between different encoding libraries, like encoding_rs and rust-encoding.

Helpful for servo/servo#13238

@hsivonen
Copy link
Owner

hsivonen commented Apr 5, 2018

This is what Encoding::name() is for. The Servo issue doesn't explain the details. Is there a reason why Encoding::name() does not address the use case?

@hsivonen
Copy link
Owner

hsivonen commented Apr 5, 2018

Also, is there a reason why rust-url can't identify the encoding by &'static Encoding while making it an opt-in feature so that apps that want to use rust-encoding don't need to pull in encoding_rs?

@hsivonen
Copy link
Owner

hsivonen commented Apr 5, 2018

@SimonSapin, is your plan from a year ago for introducing a trait to abstract over encoding libraries still your current plan for rust-url?

@talklittle
Copy link
Author

Thanks, didn't realize that it's a case-insensitive match and that there is always a corresponding label that matches the name exactly. https://encoding.spec.whatwg.org/#names-and-labels

@talklittle talklittle closed this Apr 5, 2018
@SimonSapin
Copy link

Enough desired breaking changes have accumulated that I kinda want to do url 2.0 now, but there are still some blockers and it’s a bunch of work that isn’t scheduled yet. Removing rust-encoding support would be part of that. As to the trait idea, it’s less about supporting multiple libraries than about avoiding a dependency that’s "visible" in the public APIs (and having users provide callbacks instead). cssparser does this. Servo has already migrated from rust-encoding to encoding_rs. I’m not sure if this is still a good idea to be honest…

@hsivonen
Copy link
Owner

hsivonen commented Apr 5, 2018

OK. Personally, I prefer establishing a culture of just upgrading stuff in the ecosystem even if the upgrades are in principle "breaking" changes semver-wise and, therefore, require Cargo.toml churn all around, but I can see the point of wishing to hide the concrete public type behind a trait that gets monomorphised away in order to have less Cargo.toml churn.

I foresee the following in semver-wise breaking changes for encoding_rs that aren't source-wise breaking for the current crates.io users of encoding_rs and wouldn't be for rust-url:

  1. Make the mem module assert in certain cases in debug builds and increase the minimum Rust version requirement while at it.
  2. Switch from the simd crate to nightly-only std::simd and std::arch once mask vectors with performance-wise simd crate-equivalent reductions land in the standard library's copy of std::simd. The outward breaking change would be the minimum Rust version changing.
  3. Use std::simd and std::arch unconditionally once both become available on non-nightly Rust. The outward breaking change would be the minimum Rust version changing.

Other potential sources of semver-breaking changes that wouldn't actually break the crate ecosystem source-wise:

  • Changes to the design of the mem module if the current bet for how to outperform existing Gecko code is wrong API-wise.
  • Changes to optional features around CJK encoder performance vs. binary size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants