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

search once #648

Closed
wants to merge 4 commits into from
Closed

search once #648

wants to merge 4 commits into from

Conversation

ldwgchen
Copy link
Contributor

Instead of searching the whole document in one go, search and stop at the nearest page that produces a hit.

Fixes #132. UX won't be much different, besides that when the user views a page that hasn't been searched before, they will no longer see highlighted results in this page.

@ldwgchen
Copy link
Contributor Author

ldwgchen commented Jul 29, 2024

Possible conflict with #450 (see also PR #644) since not all results are obtained at once. However, the proposed implementation does retain results in previously searched pages.

@alerque
Copy link

alerque commented Jul 29, 2024

I would consider this a UX regression. I regularly use the search as a way to view the total instances of a key word across book length texts and see the dispersion.

image

If search results were only incremental this would be much clumsier to achieve. Returning the first result and letting the UI unlock would be nice, but the remaining instances should probably be found and highlighted in the background (and an "N of X" counter in the status bar wouldn't be a bad idea either.

@laomuon
Copy link
Contributor

laomuon commented Jul 29, 2024

With the Vim-like command (/), I expect more or less it works like in Vim, i.e. search the whole document and have a counter for it. I agree with @alerque that this could be an UX regression.

@ldwgchen
Copy link
Contributor Author

ldwgchen commented Aug 1, 2024

I'm personally using zathura-pdf-mupdf. I tried opening a problematic document with mupdf and found the resulting memory usage to be quite similar to what I'd get with zathura-pdf-mupdf. Thus, I believe that the root cause of #132 likely lies upstream. Since search-once is disruptive to the common use cases mentioned by @alerque and @laomuon, it's best to look for an alternative solution and close this PR for now.

@ldwgchen ldwgchen closed this Aug 1, 2024
@ldwgchen ldwgchen deleted the search-once branch October 11, 2024 04:12
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.

Crazy memory usage
3 participants