search

kkawakam / rustyline

Readline Implementation in Rust
https://crates.io/crates/rustyline/
MIT License
1.53k stars 176 forks source link

Explore removing `SmallVec` from `Event` enum #614

Closed lopopolo closed 2 years ago

lopopolo commented 2 years ago

This PR explores removing the dependency on smallvec and replacing it with Vec from std.

smallvec is a third party dependency that leaks into the public API. If applications that depend on rustyline wish to create custom key bindings, they must also depend on smallvec to construct Event::KeySeq.

This PR replaces smallvec, which is a complicated data structure with lots of unsafe code and several known security advisories, with a vocabulary type from std.

This has the effect of reducing the size of the Event enum.

Because Event is a public enum, changing the type of the field in Event::KeySeq is a breaking change. I've shown this change in the sequence of commits in this branch and the addition of a test.

Motivation of this PR is reducing dependencies.

@gwenn if you'd prefer, I'd be willing to try an alternate PR that conditionally enables smallvec with a Cargo feature. I think it looks easy enough to support.

gwenn commented 2 years ago 
$ cargo tree
...
├── radix_trie v0.2.1
│   ├── endian-type v0.1.2
│   └── nibble_vec v0.1.0
│       └── smallvec v1.8.0
├── smallvec v1.8.0
lopopolo commented 2 years ago

😅 I am hoping to follow up with additional PRs that make the radix_trie dependency optional as well.

lopopolo commented 2 years ago

😅 I am hoping to follow up with additional PRs that make the radix_trie dependency optional as well.

I've followed up on this here: https://github.com/kkawakam/rustyline/pull/615