dsmiley commented on a change in pull request #1123: LUCENE-9093: Unified
highlighter with word separator never gives context to the left
URL: https://github.com/apache/lucene-solr/pull/1123#discussion_r361828293
##########
File path:
lucene/highlighter/src/java/org/apache/lucene/search/uhighlight/LengthGoalBreakIterator.java
##########
@@ -146,18 +182,14 @@ public int following(int followingIdx) {
return afterIdx;
}
- // note: it is a shame that we invoke preceding() *in addition to*
following(); BI's are sometimes expensive.
-
- // Find closest break < target
- final int beforeIdx = baseIter.preceding(targetIdx); // or could do
baseIter.previous() but we hope the BI implements preceding()
- if (beforeIdx <= followingIdx) { // too far back
- return moveToBreak(afterIdx);
- }
+ // note: it is a shame that we invoke preceding() *one more time*; BI's
are sometimes expensive.
- if (targetIdx - beforeIdx <= afterIdx - targetIdx) {
+ // Find closest break to target
+ final int beforeIdx = baseIter.preceding(targetIdx);
+ if (targetIdx - beforeIdx < afterIdx - targetIdx && beforeIdx >
matchEndIndex) {
return beforeIdx;
}
- return moveToBreak(afterIdx);
Review comment:
Maybe removing the moveToBreak here is an optimization attempt but it feels
wrong. Also current() accuracy will become necessary if you look at my other
comment about a small proposal to use it so this BI in preceding() doesn't get
look too far left.
It'd be nice if moveToBreak didn't need to exist. I could imagine a BI
wrapper that you could set it's position on but it wouldn't propagate. Methods
whose semantics are based on current() (like next()) would need to act
differently. Kind of a lot of code though. I took a stab at it for fun:
https://gist.github.com/dsmiley/b425b152d51d4c63f498fc84d125ea0a
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
