levenshtein: LevenshteinState, usize input distance#99
levenshtein: LevenshteinState, usize input distance#99MartinKavik wants to merge 1 commit intoBurntSushi:masterfrom
Conversation
BurntSushi
left a comment
BurntSushi
left a comment
There was a problem hiding this comment.
Thanks! Unfortunately, I don't think I will be merging breaking changes any time soon. :-(
| pub fn new( | ||
| query: &str, | ||
| distance: u32, | ||
| distance: usize, |
There was a problem hiding this comment.
Could you please say why? This seems trivial to me, and there is no way i'm going to put out a breaking change release for something like this given that I just released 0.4.
There was a problem hiding this comment.
It makes sense in combination with the main change - LevenshteinState - to make all distance types consistent.
|
|
||
| impl Automaton for Levenshtein { | ||
| type State = Option<usize>; | ||
| type State = Option<LevenshteinState>; |
There was a problem hiding this comment.
Sorry, but I'm not putting out a 0.5 release for this. :-(
Other ideas:
- Keep this PR on hold until I'm ready for another breaking change release.
- Create a new levenshtein automaton type.
- Put this automaton in a new crate.
There was a problem hiding this comment.
I would choose 1. option - on hold.
I can inline updated automaton into my project - I don't need it in the official fst release. So merge it when you want and I think this PR can be used also as an example when other users want to use automaton states.
There was a problem hiding this comment.
Great. Thanks for understanding.
src/set.rs
Outdated
| ("fozb".to_string(), Some(83)), | ||
| ("foo".to_string(), Some(LevenshteinState { state_idx: 183, distance: Some(0) })), | ||
| ("foob".to_string(), Some(LevenshteinState { state_idx: 123, distance: Some(1) })), | ||
| ("fozb".to_string(), Some(LevenshteinState { state_idx: 83, distance: Some(2) })), |
There was a problem hiding this comment.
Please wrap lines to 79 columns inclusive.
There was a problem hiding this comment.
I'll make all changed lines shorter.
FYI: When you add error_on_line_overflow = true and error_on_unformatted = true into rustfmt.toml and run cargo +nightly fmt --all, it reports 36 errors.
2 participants
Motivation
Map::search_with_state.Breaking changes
Levenshtein::new(query: &str, distance: u32)changed todistance: usize.LevenshteinState.Tests
cargo test --features levenshtein