Skip to content

feat: add snacks.picker integration #1654

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

fang2hou
Copy link

@fang2hou fang2hou commented Feb 9, 2025

Add integration support for the new picker snacks.picker, which may become the default picker in future LazyVim releases.

I have tested this integration with both single selection (for branch checkout) and multiple selection (for resetting multiple files).

sample.mp4

@fang2hou
Copy link
Author

The problem detected by linters has been fixed. (I guess 😉)

@fang2hou
Copy link
Author

Sorry, I'm not very familiar with ruby testing. I'm not sure why the e2e tests failed, but maybe restarting them could help?

@CKolkey
Copy link
Member

CKolkey commented Feb 11, 2025

Yea that looks like a git thing, not something you would have changed.

I'll try to review this soon - I have 3 month old twins at home so I'm on a bit of a forced programming break 😉

@fang2hou
Copy link
Author

@CKolkey
No worries, please enjoy being with your twins! 😙

@iniw
Copy link

iniw commented Feb 21, 2025

Hey! I was testing this out and noticed that the stash's drop picker doesn't seem to be working properly. the ones you select to drop are actually kept, and the ones that weren't selected are dropped, which is the opposite of what I'd expect it to do.

EDIT:
That doesn't seem to actually be the case. It seems to always be deleting the first one? It's weird.

@fang2hou
Copy link
Author

Hey! I was testing this out and noticed that the stash's drop picker doesn't seem to be working properly. the ones you select to drop are actually kept, and the ones that weren't selected are dropped, which is the opposite of what I'd expect it to do.

EDIT: That doesn't seem to actually be the case. It seems to always be deleting the first one? It's weird.

I compared the result from snacks and vi.ui.select, there is no difference between them. However I have also noticed the weird handling when I tried to rename my stashes, sometimes after I entered new name, the stash will be dropped! And I tested both snacks & vim.ui.select, and I believe that is the issue from Neogit itself, the fuzzy finder part is not related.

@iniw
Copy link

iniw commented Feb 24, 2025

I have also noticed the weird handling when I tried to rename my stashes, sometimes after I entered new name, the stash will be dropped! And I tested both snacks & vim.ui.select, and I believe that is the issue from Neogit itself, the fuzzy finder part is not related.

Can't reproduce this. The only weirdness I noticed is that renaming stashes doesn't focus the input box straight away, I have to <c-w>w to it, but I don't really mind that too much since It's not often that I rename stashes.

My initial comment was purely my fault, I expected that selecting multiple stashes with <tab> would drop them all, but that's not how it works :)

@mehalter
Copy link
Contributor

I have tested this out as well as just using snacks.picker by overriding vim.ui.select and I'm having problems using the diff range functionality (dr). I'm not sure if this is a bug in snacks.picker or the diff range implementation in neogit.

In the meantime, I have opened an issue here: folke/snacks.nvim#1561

@CKolkey CKolkey force-pushed the feature/snacks-integration branch from a77dd53 to 838e522 Compare March 14, 2025 12:37
@CKolkey
Copy link
Member

CKolkey commented Mar 14, 2025

The value of a proper integration beyond just overloading vim.ui.select is the ability to do stuff like this: https://github.com/NeogitOrg/neogit/blob/master/lua/neogit/lib/finder.lua#L38-L41

That allows valid git refs to be used if the input field contains certain characters, something that wouldn't be possible without overloading the "on_select" function.

Ideally a proper snacks integration would include this logic too, since otherwise it's not much more than overloading vim.ui.select (imo).

@fang2hou does that make sense, and is that something you can look into?

@iniw
Copy link

iniw commented Apr 17, 2025

I've been using this PR daily and can vouch that it works well. Would be nice for it to get merged so that I don't have to keep a fork :)

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