Skip to content

Conversation

@itsjunetime
Copy link
Collaborator

Should fix #146.

The things this does:

  1. I got rid of the word_index in the search state - it's not reasonable to keep that around and recompute it every time you enter a char just in case you want to use the control+{left,right} keys to move, so we just find the word boundaries on the fly with the unicode_segmentation crate as well.
  2. We now consider the 'first writeable column' in the search state to be 0, instead of 1 (even though 1 is technically the column number the which this string will be written in the terminal), since that just makes everything easier to reason about. We just adjust for this when we actually display the string.
  3. Simplified some test helpers, moved some things around to make it easier to process and more resilient to future refactors, etc
  4. Added more tests

@itsjunetime
Copy link
Collaborator Author

Oh yeah: the basic_dynamic_paging test messes with the terminal state and erroneously exits with a zero exit code in such a way that CI is getting both false positives and false negatives. So I need to work on that before this can function. See rust-lang/cargo#16558

@academician academician self-assigned this Feb 5, 2026
@academician
Copy link
Collaborator

Oh yeah: the basic_dynamic_paging test messes with the terminal state and erroneously exits with a zero exit code in such a way that CI is getting both false positives and false negatives. So I need to work on that before this can function. See rust-lang/cargo#16558

Hey - I played around with this and came up with this fix: itsjunetime/minus@search_with_graphemes...academician:minus:fix-dynamic-paging-test

At first I just tried changing the exit strategy, but then it was removing the exit command during init so I had to add the busy loop and sleep to ensure the pager was initialized. Not ideal, but it works. Long-term we should probably better integration tests, but hopefully this unblocks you.

@academician
Copy link
Collaborator

All in all this PR looks like a big improvement. However, it strikes me that this search input is just implementing a kind of limited, specialized version of GNU readline. If we can, long term we should replace it with something we don't have to maintain ourselves, like rustyline, reedline, dialoguer, or inquire.

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.

Panic when searching for German umlauts

2 participants