-
-
Notifications
You must be signed in to change notification settings - Fork 671
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
Collapse overlapped highlighted ranges #1473
Changes from 3 commits
23f0057
3c7b22d
3907f5a
9c5e75a
990c24b
b6d5aa6
c813c86
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -79,7 +79,7 @@ impl Snippet { | |||||
let mut html = String::new(); | ||||||
let mut start_from: usize = 0; | ||||||
|
||||||
for item in self.highlighted.iter() { | ||||||
for item in collapse_overlapped_ranges(&self.highlighted).iter() { | ||||||
html.push_str(&encode_minimal(&self.fragment[start_from..item.start])); | ||||||
html.push_str(HIGHLIGHTEN_PREFIX); | ||||||
html.push_str(&encode_minimal(&self.fragment[item.clone()])); | ||||||
|
@@ -186,6 +186,29 @@ fn select_best_fragment_combination(fragments: &[FragmentCandidate], text: &str) | |||||
} | ||||||
} | ||||||
|
||||||
/// Returns ranges that are collapsed into non-overlapped ranges. | ||||||
fn collapse_overlapped_ranges(ranges: &Vec<Range<usize>>) -> Vec<Range<usize>> { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
&Vec is almost never a good idea. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another solution would be for you to pass an iterator directly.
|
||||||
let mut result = Vec::new(); | ||||||
fulmicoton marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
let mut ranges = ranges.iter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick... I'm not fond of shadowing. Here I would either call it |
||||||
|
||||||
let mut current = match ranges.next() { | ||||||
Some(range) => range.clone(), | ||||||
None => return result, | ||||||
}; | ||||||
|
||||||
for range in ranges { | ||||||
if current.end > range.start { | ||||||
current = current.start..range.end; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick... I feel the following is easier but that's your call.
Suggested change
|
||||||
} else { | ||||||
result.push(current); | ||||||
current = range.clone(); | ||||||
} | ||||||
} | ||||||
|
||||||
result.push(current); | ||||||
result | ||||||
} | ||||||
|
||||||
/// `SnippetGenerator` | ||||||
/// | ||||||
/// # Example | ||||||
|
@@ -320,10 +343,10 @@ mod tests { | |||||
|
||||||
use maplit::btreemap; | ||||||
|
||||||
use super::{search_fragments, select_best_fragment_combination}; | ||||||
use super::{search_fragments, select_best_fragment_combination, collapse_overlapped_ranges}; | ||||||
use crate::query::QueryParser; | ||||||
use crate::schema::{IndexRecordOption, Schema, TextFieldIndexing, TextOptions, TEXT}; | ||||||
use crate::tokenizer::SimpleTokenizer; | ||||||
use crate::tokenizer::{SimpleTokenizer, NgramTokenizer}; | ||||||
use crate::{Index, SnippetGenerator}; | ||||||
|
||||||
const TEST_TEXT: &str = r#"Rust is a systems programming language sponsored by | ||||||
|
@@ -588,4 +611,35 @@ Survey in 2016, 2017, and 2018."#; | |||||
} | ||||||
Ok(()) | ||||||
} | ||||||
|
||||||
#[test] | ||||||
fn test_collapse_overlapped_ranges() { | ||||||
assert_eq!(collapse_overlapped_ranges(&vec![0..1, 2..3, ]), vec![0..1, 2..3]); | ||||||
assert_eq!(collapse_overlapped_ranges(&vec![0..1, 1..2, ]), vec![0..1, 1..2]); | ||||||
assert_eq!(collapse_overlapped_ranges(&vec![0..2, 1..2, ]), vec![0..2]); | ||||||
assert_eq!(collapse_overlapped_ranges(&vec![0..2, 1..3, ]), vec![0..3]); | ||||||
} | ||||||
|
||||||
#[test] | ||||||
fn test_snippet_with_overlapped_highlighted_ranges() { | ||||||
let text = "abc"; | ||||||
|
||||||
let mut terms = BTreeMap::new(); | ||||||
terms.insert(String::from("ab"), 0.9); | ||||||
terms.insert(String::from("bc"), 1.0); | ||||||
|
||||||
let fragments = search_fragments(&From::from(NgramTokenizer::all_ngrams(2, 2)), text, &terms, 3); | ||||||
|
||||||
assert_eq!(fragments.len(), 1); | ||||||
{ | ||||||
let first = &fragments[0]; | ||||||
assert_eq!(first.score, 1.9); | ||||||
assert_eq!(first.start_offset, 0); | ||||||
assert_eq!(first.stop_offset, 3); | ||||||
} | ||||||
|
||||||
let snippet = select_best_fragment_combination(&fragments[..], text); | ||||||
assert_eq!(snippet.fragment, "abc"); | ||||||
assert_eq!(snippet.to_html(), "<b>abc</b>"); | ||||||
} | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add your example in the comment.