Skip to content
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

Fix bug 'DefaultPassageFormatter.format' method #13832

Merged
merged 7 commits into from
Oct 3, 2024

Conversation

Seunghan-Jung
Copy link
Contributor

Description

In the following PR (#13276), I added an option to allow customizing the sorting of Passages when using the UnifiedHighlighter. As a result, it is no longer guaranteed that the startOffset of the next passage will be greater than that of the previous passage. However, I am now modifying the DefaultPassageFormatter.format() method, where a conditional statement was written under the assumption that this was guaranteed. You can refer to the test code for examples and better understanding.

Copy link
Contributor

@dsmiley dsmiley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just needs a CHANGES.txt entry. No more 9.x releases so this will be in 10.

@Seunghan-Jung
Copy link
Contributor Author

@dsmiley
I have added the relevant entry to the CHANGES.txt at commit, 2b2e8a8

@dsmiley dsmiley merged commit e3e3328 into apache:main Oct 3, 2024
3 checks passed
@@ -270,6 +270,9 @@ Bug Fixes
* GITHUB#12878: Fix the declared Exceptions of Expression#evaluate() to match those
of DoubleValues#doubleValue(). (Uwe Schindler)

* GITHUB#13832: Fixed an issue where the DefaultPassageFormatter.format method did not format passages as intended
when they were not sorted by startOffset. (Seunghan Jung)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is misplaced. 10.0.0 is feature frozen. It should go to either 10.0.1 or 10.1.0

@javanna
Copy link
Contributor

javanna commented Oct 3, 2024

@dsmiley 10.0 is feature frozen, a vote is ongoing. this does need to be backported to branch_10x though it seems, and the changes entry moved to the right place. Please add the appropriate milestone as well?

@dsmiley dsmiley added this to the 10.1.0 milestone Oct 3, 2024
@dsmiley
Copy link
Contributor

dsmiley commented Oct 3, 2024

Oops; thanks for the guidance. It's been a while since I committed/merged in Lucene. I'll move to 10.1.0.

dsmiley pushed a commit that referenced this pull request Oct 4, 2024
…assages (#13832)

The ellipsis should have been inserted in more scenarios.

(cherry picked from commit e3e3328)
@dsmiley
Copy link
Contributor

dsmiley commented Oct 4, 2024

I fixed CHANGES.txt and backported to branch_10x now.

@javanna
Copy link
Contributor

javanna commented Oct 4, 2024

Thanks @dsmiley !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants