-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add boolean toggle value with increment/decrement shortcuts #2977
Conversation
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 :) |
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. 7f05340 |
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! |
I agree about it being a different concept than inc/dec and can see it leading to weird behaviors like [[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 |
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! |
fixes #2931