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

Add a new prompter for meow-visit. #501

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

okamsn
Copy link
Contributor

@okamsn okamsn commented Sep 3, 2023

These changes make type what is sought in meow-visit more like Isearch and Anzu. It is still a work in progress, but it seems to be working.

The change adds a user option to configure a "prompter" which is used to read text from the minibuffer. It also adds a new prompter that highlights matches from point to the top or bottom of the window, depending on the argument to meow-visit. If point is inside the secondary selection, then matches within the selection are also highlighted. I find this useful for running meow-visit to place the beacons when meow-visit-sanitize-completions is nil.

Are you interested in adding something like this? If so, what would you like to see changed?

@okamsn okamsn marked this pull request as draft September 3, 2023 16:13
@okamsn okamsn force-pushed the visit-preview branch 3 times, most recently from 5310f5d to b9e9747 Compare September 3, 2023 16:47
- Add user option `meow-vist-prompter`.

- Add `meow--prompt-buffer-highlight`, which highlights matches in the buffer
  similar to Isearch, and highlights all matches in the secondary selection
  while in Beacon Mode and point is in the secondary selection.
@DogLooksGood DogLooksGood marked this pull request as ready for review September 3, 2023 17:42
@DogLooksGood DogLooksGood self-requested a review September 3, 2023 17:42
@DogLooksGood
Copy link
Collaborator

Hey, I just gave it a try, and it looks cool. The buffer-highlight style seems to be good and I'd like to have this change.

BTW, should the window view scroll to the occurrence which is not in current window view?

@eshrh
Copy link
Member

eshrh commented Sep 12, 2023

Very cool! I still prefer to use ivy over visit/anzu/isearch but this is a good improvement to vanilla meow.

I skimmed the code and it seems good. One thing I think would be nice is a more code reuse to make it easier to write new prompters.

  • Putting useful functions like transform-text in the flet means reuse if you're making another prompter. I haven't looked too hard at the regexs in completion and buffer-highlight but surely there's some code reuse that can happen there.
  • Your beginning/ending pos code could also be easily reused, can easily be moved into the parent let*
  • Basically making the call to each prompter as simple as possible, and documenting what it receives and should return.

BTW, the regex's don't seem like they're doing the same thing, i'll take a closer look soon. I noticed that default doesn't seem to get words broken up by hyphens in lisp code. This could be a good chance to refactor the default prompter.

@okamsn
Copy link
Contributor Author

okamsn commented Sep 17, 2023

I'm now thinking about whether the new other way of doing it could just use Isearch, instead of me re-inventing a worse Isearch with less understanding.

Would something like a user option for a function that returns the beginning and end of the match be acceptable? I see that in #175 there is talk about how to integrate with other search packages. I can check what level of effort would be needed to work with the other packages.

@VitalyAnkh
Copy link
Member

@okamsn How's the status of this PR now?

@DogLooksGood
Copy link
Collaborator

DogLooksGood commented Oct 22, 2024

There's another PR(#560) to replace the search functionality in meow with ISearch with some tight integration. I modified a little bit(address some conflicts) and put it in a branch isearch. Generally I think it's a better approach than implementing a new search. However there are still some issues unsolved, I left them in the commit message.

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.

4 participants