Removed encode-unicode dependency.#125
Closed
markhildreth wants to merge 1 commit intophsym:masterfrom
Closed
Conversation
The reason for attempting to remove this is that encode-unicode provides
some additional iterators that can make some type inferences break by
including it (or a crate that depends on it, like prettytable-rs).
For example, the following line of code works as expected if you include
the `rand` crate:
```
use rand::thread_rng;
use rand::distributions::{Distribution, Standard};
fn main() {
let mut rng = thread_rng();
let v: Vec<u8> = Standard.sample_iter(&mut rng).take(5).collect();
println!("{:}", v);
}
```
However, once prettytable-rs is included and used, the types will no
longer be able to be inferred.
```
error[E0282]: type annotations needed
--> src/main.rs:16:31
|
16 | let v: Vec<u8> = Standard.sample_iter(&mut rng).take(5).collect();
| ^^^^^^^^^^^
| |
| cannot infer type for type parameter `T`
```
While not technically a problem that NEEDS to be fixed (type inference
breaks are not really considered "breaking changes, it can be a pain
point. Seeing that prettytable-rs doesn't seem to make much use of the
additional items in encode-unicode, perhaps it's not worth the pain to
users to include it.
Codecov Report
@@ Coverage Diff @@
## master #125 +/- ##
=======================================
Coverage 85.89% 85.89%
=======================================
Files 5 5
Lines 1326 1326
=======================================
Hits 1139 1139
Misses 187 187
Continue to review full report at Codecov.
|
Collaborator
|
Repro has different issue with now stable @ 1.65: $ cargo run Switching to appropriate cargo run This was fixed in upstream I think - closing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The reason for attempting to remove this is that encode-unicode provides
some additional iterators that can make some type inferences break by
including it (or a crate that depends on it, like prettytable-rs).
For example, the following line of code works as expected if you include
the rand crate:
However, once prettytable-rs is included and used, the types will no
longer be able to be inferred.
The reason that this occurs is because prettytable includes encode-unicode, which itself adds additional ways that a
Vec<u8>can be created (specifically from anIterator<Utf8Char>) Since Rust cannot tell ifStandard.sample_itermight return anIterator<Utf8Char>vs anIterator<u8>, it cannot provide type inference.To be sure, this is not technically a bug, nor does it even need to be fixed. Type inference breaks are not really considered "breaking changes", but it can be a pain
point. Seeing that prettytable-rs doesn't seem to make much use of the
additional items in encode-unicode, perhaps it's not worth the pain to
users to include it.