Skip to content

Comments

feat(pick): update event and window config #2276

Closed
somnam wants to merge 3 commits intonvim-mini:mainfrom
somnam:feat/pick-window-config-and-update-event
Closed

feat(pick): update event and window config #2276
somnam wants to merge 3 commits intonvim-mini:mainfrom
somnam:feat/pick-window-config-and-update-event

Conversation

@somnam
Copy link

@somnam somnam commented Feb 19, 2026

Following the discussion in #2268. This PR attempts to expose minimal information allowing for custom modifications to mini.pick based on triggering an event on pick update. It also exposes window config calculation so it may be altered by the custom modification.

@somnam somnam marked this pull request as ready for review February 19, 2026 16:57
@echasnovski
Copy link
Member

Thanks for the PR!

Couple of observations:

  • My immediate reaction is that MiniPickUpdate is too vague for the event. And the "... just after picker UI is updated (on query change, cursor move, window resize, etc.)." kind of confirms that. I was thinking more in terms of "event on whenever current item is changed". Would this solve the problem? I guess the more broad MiniPickUpdate is also useful for adjusting window config of a side preview, but I'd guess it can be resolved by other means maybe.

    Not strongly opposed to this broad of event, but would like to try to make change more useful that for side-by-side preview.

  • Why is there a need for H.inside_minipickupdate conditioning? This is another example of why this event might be too broad.

  • There will be no new exported function specifically for the "side-by-side preview". Why is get_window_config() is needed?

At this point, I'd also like to ask to see how the minimal (i.e. not fancy) implementation of the side-by-side preview can look like with the proposed changes to the 'mini.pick' code. So that there won't be any new additional PRs to cover this use case.


Also, @somnam, I would like to point out that it is better to first create a feature request issue before the PR. This usually saves time for both parties (PR author and reviewer(s)) by first discussing the possible scope without having to add/review tests+documentation. It is documented in 'CONTRIBUTING.md'.

@somnam
Copy link
Author

somnam commented Feb 22, 2026

My immediate reaction is that MiniPickUpdate is too vague for the event.

I did notice that H.picker_update function is called often. As you've suggested I've tried a combination of other events and it seems that MiniPickStart, MiniPickMatch, VimResized, MiniPickStop and custom mappings for cursor movement covers all the cases for refreshing a preview window. This new event probably isn't needed.

Why is get_window_config() is needed?

I've used this to calculate the dimensions of the preview window and adjust the picker window accordingly. Turned out that fetching the config via nvim_win_get_config at MiniPickStart and VimResized allows to adjust the picker window size and position. This probably isn't needed also.

Also, @somnam, I would like to point out that it is better to first create a feature request issue before the PR.

Sorry, if I caused any inconvenience. The posted PR was basically working and I was using it for a few days, so I thought why not. I will keep this in mind for the future.

Thanks for all the feedback. I'll close this PR as it seems that the current events are sufficient for a side-by-side preview w/o timers.

@somnam somnam closed this Feb 22, 2026
@echasnovski
Copy link
Member

MiniPickStart, MiniPickMatch, VimResized, MiniPickStop and custom mappings for cursor movement covers all the cases for refreshing a preview window.

Yeah, these are probably enough. Replacing "custom mappings fro cursor movement" with custom event (like MiniPickMove) was what I earlier described as "make less painful". But yeah, if it can be avoided, I'd rather not add that.

Sorry, if I caused any inconvenience. The posted PR was basically working and I was using it for a few days, so I thought why not. I will keep this in mind for the future.

No worries. Just wanted to mention in case of future contributions.

Thanks for all the feedback. I'll close this PR as it seems that the current events are sufficient for a side-by-side preview w/o timers.

I think several people would be curious to look at the code (if you'd okay with sharing it, of course). The "Show and Tell" discussions are there specifically for this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants