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

Zk Grep #171

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Zk Grep #171

wants to merge 8 commits into from

Conversation

shfattig
Copy link

Use Telescope's "jobs" to perform live asynchronous calls to zk list -m to provide grep-like searching of note bodies

if the 'grep' argument is provided, the ui is called directly.
the api is not called with the ui as a callback (as would normally be
the case) because in this case the ui has to call the api on each input
change
run grep picker if 'grep' is given, otherwise run the other one
picker uses asynchronous calls to "zk list -m <search_term>" to live filter notes based on a search term
implemented based on Telescope's live_grep
does not include ability to jump to line since "zk list" does not give
this info
@shfattig
Copy link
Author

To test, run :ZkGrep

@shfattig
Copy link
Author

closes #127

@shfattig
Copy link
Author

shfattig commented Jun 22, 2024

A few things to clean up, but should be functional right now. Additionally, as mentioned in #127 , zk list -m does not provide line numbers of matches, but doing so would greatly enhance the match preview as well as the opening of the file. Also, zk list -m seems to behave differently from zk list -i, matching less frequently and omitting expected matches for some reason

edit: compare to zk list -i (although zk edit -i seems to give model behavior as well)

@tjex
Copy link
Member

tjex commented Jun 22, 2024

Tested by replacing the hard coded notebook root with my own directory. I can execute the command and grep :)
Great!

I'm going to have a look at how telescope grepper does the highlighting for the returned grep result in the previewer and see how trivial it is to implement. Thanks for this pr! 🙌

@tjex
Copy link
Member

tjex commented Jun 22, 2024

A few things to clean up, but should be functional right now. Additionally, as mentioned in #127 , zk list -m does not provide line numbers of matches, but doing so would greatly enhance the match preview as well as the opening of the file. Also, zk list -m seems to behave differently from zk list -i, matching less frequently and omitting expected matches for some reason

edit: compare to zk list -i (although zk edit -i seems to give model behavior as well)

It would seem that zk list -i only searches through note titles? This is why it's returning less results than zk list -m <term>.

Jumping to the line number would be great. I guess we can also look to the Telescope grep implementation (as well as highlighting) here as well.

@shfattig
Copy link
Author

Tested by replacing the hard coded notebook root with my own directory. I can execute the command and grep :) Great!

I'm going to have a look at how telescope grepper does the highlighting for the returned grep result in the previewer and see how trivial it is to implement. Thanks for this pr! 🙌

You're welcome! Yeah it should be fairly trivial as long as we can get match line numbers. I'll work on some of the cleanup (like removing that hard coded path) and attempt a call to the lua api instead

Copy link

This pull request has been automatically marked as stale because it has not had recent activity.

@github-actions github-actions bot added the stale label Jul 23, 2024
@lingling9000
Copy link

Any updates on this feature? I would love to have this!

@github-actions github-actions bot removed the stale label Aug 10, 2024
@tjex tjex added the help wanted Extra attention is needed label Aug 21, 2024
@shfattig
Copy link
Author

Any updates on this feature? I would love to have this!

Haha thanks for the poke. I'm looking at using the zk lua api instead of an external shell command for the grepping. It doesn't look like Telescope has an existing finder for this, so I'll have to figure out how to get there.

I did find that changing the matching strategy to "re" (regex) does give better results (rather than waiting until the end of a word). It also gives more intuitive usage haha, given that this is a grep feature after all...) - what do you think?

@shfattig
Copy link
Author

Other than that, maybe we should outline what the minimum requirements are to get this merged in. I might argue that it's functional as is and ready for public release, as long as code style is fine and docs are updated appropriately (I can go do that, but you might have to help me find the right places)

@shfattig
Copy link
Author

@tjex can you get us line numbers on zk list -m?

@tjex
Copy link
Member

tjex commented Aug 26, 2024

I have a bit too much to tackle at the moment both here with zk (mainly new docs site, another active PR and some aging bugs) and my study / job hunting commitments to be able to deliver something like that in any respectable amount of time :(

I would personally be fine with merging without the jump-to-line feature. You could also leave a comment where that step should be implemented, so that it can be addressed later. Better to have the cake, even if the cherry on top is missing for the moment imo.

Another approach could be to grab the user input search query, and after entering the file, executing a regular buffer neovim search, using the user input as the search term?

@tjex
Copy link
Member

tjex commented Aug 30, 2024

@shfattig Maybe there is something to take from this discussion on zk-org/zk.

zk-org/zk#330

This person was also looking to integrate jump to line from grep / fzf. Perhaps it works for your contribution as well.

@shfattig
Copy link
Author

@shfattig Maybe there is something to take from this discussion on zk-org/zk.

zk-org/zk#330

This person was also looking to integrate jump to line from grep / fzf. Perhaps it works for your contribution as well.

Yeah, interesting. I can explore that more. I still think this feature is worth merging as is, and getting it in front of people might help to refine the behavior we're looking for. I'll be able to take some time for this in a couple weeks

grep option insertion, instead of overwrite
@tjex
Copy link
Member

tjex commented Sep 25, 2024

I'm having a little look at it too, cleaned up a few things just now.
Non-telescope users will get a nasty error when running this too. Should be addressed, as the command for the other pickers is not going to come around too quickly..

Copy link

This pull request has been automatically marked as stale because it has not had recent activity.

@github-actions github-actions bot added the stale label Nov 28, 2024
@tjex tjex removed the stale label Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants