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

xtext: getSelectedWords returns with indices #2013

Merged
merged 2 commits into from
Jan 3, 2025

Conversation

TnS-hun
Copy link
Contributor

@TnS-hun TnS-hun commented Dec 27, 2024

Rename getSelectedWords to getSelectedWordIndices, and return the word boundary indices instead of the words. This is needed for highlighting. The words themselves can be obtained by passing the indices to the getText function.

Warning! Breaking change.


This change is Reviewable

Copy link
Contributor

@poire-z poire-z left a comment

Choose a reason for hiding this comment

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

Fine with me. I'll review the frontend PR later.

xtext.cpp Outdated
// Get (as a single UTF-8 string) the segment of m_text, extended to include
// the full words that may be cut at boundaries (start, end).
void getSelectedWords(int start, int end, int context) {
// Get extended start and end indices that include the full words that may be cut at boundaries (start, end).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Get extended start and end indices that include the full words that may be cut at boundaries (start, end).
// Get start and end indices of an extended segment of m_text that includes the full words that may be cut at boundaries (start, end).

(Just for clarity, "extended indices" reads odd to me.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the comments.

Rename getSelectedWords to getSelectedWordIndices, and return the word boundary indices instead of the words. This is needed for highlighting. The words themselves can obtained by passing the indices to the getText function.

Warning! Breaking change.
@TnS-hun TnS-hun force-pushed the getSelectedWordIndices branch from 2fb4f9e to 734eb01 Compare December 27, 2024 18:42
@TnS-hun
Copy link
Contributor Author

TnS-hun commented Dec 28, 2024

I introduced and off by one error, so I added a small fix commit. (I did not want to use force push because it is two character change.)

@Frenzie
Copy link
Member

Frenzie commented Dec 28, 2024

We'll just squash it. :-)

@Frenzie Frenzie merged commit c844e4b into koreader:master Jan 3, 2025
4 checks passed
@poire-z
Copy link
Contributor

poire-z commented Jan 3, 2025

Warning! Breaking change.

You may have been too quick at merging this :/
If you need to bump base into frontend, we'll need to either finalize #12948, or an interim commit to just translate bits to the xtext api change.

@Frenzie
Copy link
Member

Frenzie commented Jan 3, 2025

Hm, indeed. If necessary I'll force push or revert.

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