helix

helix copied to clipboard

fixes #2931

fixes #2931

I was tackling this as my first issue and got stumped yesterday haha. Glad I checked PRs and can move to new stuff. Nice code, def learned smthn.

@CBeige you can always ask for help in the matrix room :)

fixes #2931

I was tackling this as my first issue and got stumped yesterday haha. Glad I checked PRs and can move to new stuff. Nice code, def learned smthn.

@CBeige Sorry, I should have claimed the issue before starting working on it.

Can I have a review please?

Code-wise this looks fine to me but IMO toggling a boolean is conceptually different from increment/decrement. Even as a standalone binding or feature I think this is too specific: I don't see much value over editing the boolean by hand.

I have been using this PR for a while now, and find it particularly handy when dealing with config files. Where the main activity is tweaking a value or flipping a switch. For me it became a no-brainer quickly to move thru a file pressing C-a to change a setting.

I understand your concern about it being "conceptually different", however there are only so many keybindings to remember and using C-a to scroll thru an ordered enumeration would make sense to me. Eg [0,1,2,3,...], [true,false], [yes,no], [a,b,c,...], [red,green,blue,...16 base colors/ or colors already in use in eg a theme file]. Agreed it wil not always make sense to use this feature, however it would be up to user to choose what feature to use/apply when?

So I took this code and changed it around a bit.. so it can work on lists given in the config file. https://github.com/helix-editor/helix/commit/7f053408ed11f0432cf1cd6e29c9a5cb7334f190

I think it is the same concept. You are walking through a list of values like aiko mentions. The only difference is there are only two possible values, but it is still going "up" and "down".

Btw, I had the same idea soon after I learned the dec/increment tool, and even tried it on a bool expecting it would work, so I guess it's quite intuitive

hope this get merged soon!

Code-wise this looks fine to me but IMO toggling a boolean is conceptually different from increment/decrement. Even as a standalone binding or feature I think this is too specific: I don't see much value over editing the boolean by hand.

I agree about it being a different concept than inc/dec and can see it leading to weird behaviors like "#<C-a> flipping every other selection. I think it could possibly pull its weight though as a separate, more general, toggle action (Ctrl-t?), with the toggles defined per-language, e.g.

[[language]]
name = "rust"

[language.toggle-items]
"true" = "false"
"==" = "!="
"&&" = "||"
# .. and inverses

Having it language-specific means you can develop a habit that works across languages without having to remember which ones use !=, ~=, <>, /=, etc.

I am closing this PR as stale. It has significant conflicts and I don't see us going forward with this. We want to keep the builtin increment functionality simple and only support simple base10 integers.

There are a wide variety of formats and literals that will be language dependent and potentially complex to parse. That kind of functionality belong in plugins (or can even be implemented with pipes today).

Thank you for contributing!