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

[Feature Request]: fire user event on pin buffer #857

Closed
jackielii opened this issue Jan 31, 2024 · 8 comments
Closed

[Feature Request]: fire user event on pin buffer #857

jackielii opened this issue Jan 31, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@jackielii
Copy link
Contributor

What?

Dispatch a user event when pinning a buffer. I.e. add something like:

  vim.api.nvim_exec_autocmds("User", { pattern = "BufferlinePinBuffer" })

to groups.persist_pinned_buffers

Why?

I'd like to create a Neotree source that shows pinned buffers, in part to work around #445

In order to refresh the view, I need to listen on an event. Dispatching a user event from bufferline seems to be the right solution?

@jackielii jackielii added the enhancement New feature or request label Jan 31, 2024
@akinsho
Copy link
Owner

akinsho commented Feb 1, 2024

🤔 so generally this isn't something that bufferline really does. It's not a bunch of code by any means but it does feel like the beginning of a precedent that might need just a bit of thinking through. I'd really like to avoid a torrent of requests for even more events.

I don't necessarily think it's a bad approach just requires a bit of mulling over. I wonder if a neotree source is something that should exist. Bufferline is already a UI plugin so it needing yet another UI plugin to show part of it's UI seems or like a gap that this is filling in for.

Feels like the right solution is that pinned buffers remain visible rather than this workaround. As I mentioned there though that's a fair bit of work that I don't currently need and really not up for spending several days on working this out despite not personally having this use case currently

@jackielii
Copy link
Contributor Author

Hi @akinsho , fair point. It does feel if this approach is taken, a lot of the actions will also want to emit events, maybe?

Just to re-iterate my use case: When I'm in a large project, switching around by jumping to definition or grep a string, I quickly get lots of files open. At any point, I want to jump to the few "main" files that I'm editing, usually 2 or 3, without thinking about it. Harpoon does this exactly and I tried to integrate bufferline with Harpoon: ThePrimeagen/harpoon#352 (comment), but I quickly realised, bufferline pinned buffer + persistent session give me 90% what I need and won't cause any hot key conflict.

If pinned buffers can be seen all the time, then my use case is mostly covered. Even though I quite like showing the pinned buffers in the left panel. See screenshot here: https://github.com/jackielii/neo-tree-bufferline.nvim.

I don't mind any solution, efficient or not, that allows me to hook into the change of bufferline. Any existing event that already does this?

@akinsho
Copy link
Owner

akinsho commented Feb 1, 2024

@jackielii so I think what you're trying to do is also a problem I've encountered/use case I've seen but I personally don't actually think of bufferline as the layer where this needs to be solved. Plugins like harpoon exist for maintaining a list of buffers you are curating e.g. a list of tests etc. I know that with groups and pins bufferline sort of gets into that world but I'd say the core use case stretches past what bufferline is here for. Imagine you had 10 pinned buffers you wanted to track and they all remained visible and other open buffers weren't or maybe it's way past 10 like 40. Whatever the number this clearer is a problem that is no longer well suited to the tab bar at the top of you editor.

It feels like a subset of buffer management/handling which though this plugin intersects with that because of some of the UI features I've added it really isn't intended to do. Pinned buffers remaining visible is one thing in the context of keeping one or two things always visible but when it becomes a mechanism for managing a bunch of buffers I don't think it's the right solution.

Seems like a better solution would be a neotree source for harpoon? since harpoon specifically exists for marking buffers? there are other such plugins but I personally haven't found any I like which is probably why some minimal version of this idea crept in here but I don't think I want this plugin to go fully down that path

@jackielii
Copy link
Contributor Author

I agree there are obvious problems for pinned buffers to always show. So we're in agreement on this point.

The reason I gave up on Harpoon is I have <M-1> ... <M-9> for jumping through the buffers. I pin a buffer and a naturally reach it via <M-1> for the first pinned buffer. When I bring Harpoon in, I initially set <M-1> ... <M-4> for harpoon, <M-5>...<M-9> for bufferline. It gets confusing very quickly: Am I reaching for harpoon or buffers? My mind had to make this distinction before pressing the button.

Bufferline's pinned buffers works great for me because it's always at the beginning of the indices of the buffers. Isn't this part of your original design goal? Plus I can remove all non-pinned buffers easily.

The only problem with this is I forget what buffers are pinned. I used to switch to first buffer to check and switch back. So displaying them in the left panel where I usually have the tree always open, I can spare a bit space to have pinned buffers always display seems to be a great solution.

image

I can understand why you don't want to go down this direction. But I argue this is generally how software develops? If we find enough use cases, then we abstract and find a generic solution for all similar problems. Before we get there, this is a valid little use case to test out how well it works. If later we find it doesn't work out, remove it or upgrade it with a different approach?

Thanks & Regards,

@jackielii
Copy link
Contributor Author

I understand the current PR for this issue is not a good one. Would you consider a better PR in this direction?

A high level idea is:

  1. expose an option to control whether to emit a event
  2. have a event emitter for most events that updates state

If you're totally against this idea which is perfectly fine. I will close the issue and the PR.

Thanks and regards,

@akinsho
Copy link
Owner

akinsho commented Feb 7, 2024

Tbh this isn't on the surface technically a bad idea but I think a few general things are important to note. In general most of my work now on this plugin is about managing complexity. Arguably a plugin like this could have been written in 20% the lines of code but over time I've definitely compromised too much to add functionality/complexity for individual user's use cases. This has made the job of maintaining this plugin significantly less pleasant as I'm fixing code I didn't want to write or add for use cases I don't have/barely understand.

Apologies for the overly long response but I'm hoping it really clarifies my reluctance and might do for someone else in the future. For me any decision that makes maintaining this plugin more tolerable in the long term is the best decision. I think your usecase whilst valid is so specific to your workflow and preferences and the moment this is added people will start using it and it will become a new unwanted surface area for maintenance (this has happened to innumerably since I created this plugin, it always goes this way) so I think it's best to not add this.

If more people pile in over time and more and more use cases come in then maybe but at that point it will largely be a noise of feature requests vs burden of maintenance calculation.

@jackielii
Copy link
Contributor Author

Hi @akinsho , Thanks for the thorough explanation. I can totally understand.

@jackielii
Copy link
Contributor Author

BTW, I found a workaround by patching the ui.refresh method. I agree too many features unrelated to the core functionality is not a good idea. But I'm glad that these kind of work around exists to accommodate my use cases. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants