Skip to content

Add labelled buffer picker #13177

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 1 commit into
base: master
Choose a base branch
from

Conversation

leath-dub
Copy link

This is akin to https://github.com/leath-dub/snipe.nvim. You essentially have a dictionary of characters which are used to generate tags for the currently open buffers. You can see attached the effect with many open buffers:

20250323_22h01m29s_grim

Looking at the screenshot example, if you press the key sequence sg, it will bring you to the buffer with helix-core/src/editor_config.rs open in it.

The changes extend the Picker struct to allow you to disable the prompt and have custom key event handling, while also adding a new command for the labelled buffer picker. Wasn't really sure how to handle extending the picker to not allow for a prompt.

This is draft, as I would love some feedback around the code, I am not a rust dev regularly and I am of course new to the helix codebase (so far well done btw ! - it is very nice to navigate).

I also understand this is more of a personal scratch to itch, but I thought I would share in case you want the changes also.

@leath-dub leath-dub force-pushed the labelled-buffer-picker branch from f164253 to b2bea65 Compare March 23, 2025 23:14
@nik-rev
Copy link
Contributor

nik-rev commented Mar 23, 2025

So this is like gw but for buffers? WOW, that's really cool!

impl Default for BufferPickerConfig {
fn default() -> Self {
Self {
label_alphabet: "sadflewcmpghio".chars().collect(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't really an "alphabet"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just using the same terminology that is already used for the gw feature. It is called jump_label_alphabet.

@leath-dub leath-dub force-pushed the labelled-buffer-picker branch from b2bea65 to d2264d8 Compare March 24, 2025 11:29
@leath-dub
Copy link
Author

Just pushed some fixes there, not sure what the etiquette around resolving review comments (can the reviewee click the resolve button?).

Apart from fixes according to the comments by @nik-rev, I also realised I was using a string unecessarily and fixed that.

@leath-dub leath-dub force-pushed the labelled-buffer-picker branch from d2264d8 to 2a5b74f Compare March 24, 2025 12:00
@nik-rev
Copy link
Contributor

nik-rev commented Mar 24, 2025

Just pushed some fixes there, not sure what the etiquette around resolving review comments (can the reviewee click the resolve button?).

Apart from fixes according to the comments by @nik-rev, I also realised I was using a string unecessarily and fixed that.

the comments are just some small things that popped into my mind, feel free to resolve any of them. Great work on this PR!!

@leath-dub
Copy link
Author

One thing I wasn't sure about was what to do when the user types the wrong characters. Right now I just have the key recording reset.

@nik-rev
Copy link
Contributor

nik-rev commented Mar 24, 2025

I can't compile your PR

helix is on 1.76

image

@leath-dub
Copy link
Author

I can't compile your PR

image

Ooh what version of rust does helix use? I think I am on nightly

@nik-rev
Copy link
Contributor

nik-rev commented Mar 24, 2025

1.76 as specified in rust-toolchain.toml

@leath-dub
Copy link
Author

1.76 as specified in rust-toolchain.toml

cool, Ill fix that now.

@RoloEdits
Copy link
Contributor

Helix tracks to firefox's MSRV. use was introduced in 1.82.0, which just so happens to be the next (estimated) MSRV, which is april first. Might be fine to leave this for now, as the MSRV for helix would support that in 8 days (as long as the capture rules still work).
image

@leath-dub
Copy link
Author

Helix tracks to firefox's MSRV. use was introduced in 1.82.0, which just so happens to be the next (estimated) MSRV, which is april first. Might be fine to leave this for now, as the MSRV for helix would support that in 8 days (as long as the capture rules still work). image

I will check if I fix the issue on the old version that it still compiles on latest, and if it does I think we should just target the older version.

@leath-dub leath-dub force-pushed the labelled-buffer-picker branch from 2a5b74f to 5c7a960 Compare March 24, 2025 23:20
@leath-dub
Copy link
Author

OK so it works on helix's toolchain and 1.85.0 (latest) toolchain too.

@leath-dub leath-dub marked this pull request as ready for review March 28, 2025 14:27
@leath-dub
Copy link
Author

I have been using this branch myself for the past few days and have not had any issues, so I changed it from draft just there - I am pretty happy that there are no glaring issues

@the-mikedavis
Copy link
Member

IMO this fits better in the plugin space. I don't want competing workflows for basic stuff like switching buffers. (I also don't like the bufferline and the feature creep that has caused, for comparison.)

@leath-dub
Copy link
Author

IMO this fits better in the plugin space. I don't want competing workflows for basic stuff like switching buffers. (I also don't like the bufferline and the feature creep that has caused, for comparison.)

I agree somewhat in that there are features already in helix that feel like they should be plugins. I guess the question is whether you want to only have some things in core until plugin support comes and you can test the breadth of API by moving these features into plugins. This is kinda what I assumed with some of the features like gw and bufferline

@leath-dub
Copy link
Author

IMO this fits better in the plugin space. I don't want competing workflows for basic stuff like switching buffers. (I also don't like the bufferline and the feature creep that has caused, for comparison.)

I agree somewhat in that there are features already in helix that feel like they should be plugins. I guess the question is whether you want to only have some things in core until plugin support comes and you can test the breadth of API by moving these features into plugins. This is kinda what I assumed with some of the features like gw and bufferline

Edit: this is also a good candidate as it is built on top of the built-in selection menu.

@the-mikedavis
Copy link
Member

There are things that could eventually be moved out into "tier 1" plugins (i.e. shipped with the editor, same as Kakoune's scripts like windowing) like the EditorConfig integration. I don't want to accept features that we would eventually remove entirely though and I don't see us maintaining this as a kind of "tier 1" plugin in the long run. Similar with bufferline and in retrospect I wish we hadn't merged it to begin with.

@leath-dub
Copy link
Author

There are things that could eventually be moved out into "tier 1" plugins (i.e. shipped with the editor, same as Kakoune's scripts like windowing) like the EditorConfig integration. I don't want to accept features that we would eventually remove entirely though and I don't see us maintaining this as a kind of "tier 1" plugin in the long run. Similar with bufferline and in retrospect I wish we hadn't merged it to begin with.

I understand, I will maintain this as a fork anyway. It is a small feature overall, but I understand feature creep happens bit by bit - rarely big changes.

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