-
-
Notifications
You must be signed in to change notification settings - Fork 920
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 description to key binds #6358
Conversation
- add description attribute to SKeybind - changed handeBind accordingly
why not add a flag for description like Also, this will break with descriptions with |
Good idea, to get ridge of " inside the key bind itself
Yeah. had that in the back of my mind too. First reaction was to just use the docs to let the users know they shouldn't use the delimiter in the description. |
yes, because we limit the amount of arguments to 4, so anything after the 3rd Also, no need to ping, I have notifications on, you know. |
Not used anywhere but should be there for good measure
I've added hyprctl functionality too now but I have a question on how you want the description to be handled.
Pro of the lather: Would be more in line with how it should be in my opinion, especially with the JSON out. The current implementation is the first. |
for json you can omit the k-v if it's not set, but it doesn't matter. Request review when you've done it the way you prefer |
then it's fine with me 👍 I did the docs real quick too so I would say this PR and the docs PR are ready to review. |
* changed hasDescriptions to hasDescription
Alright, let's try this again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
I get an invalid dispatcher error when firing up hyprland from this commit with:
specifically with |
I'm not sure here, but there really is no dispatcher called |
mouse dispatcher |
Yep same |
Oh yeah, my bad. Found it. I did a push to my fork and I can also provide a patch if you want. |
Describe your PR, what does it fix/add?
I'm bad at remembering key binds and was always a fan of nvim plugins like which key. For something like this, a way of describing your key binds is necessary.
My proposal would look something like this:
but obviously, it should be backward compatible so this should also be no problem:
Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)
Working with any bind the description would always come after the KEY (so at index 2) and the indicator would be double quotes. I think some kind of indicator is needed to distinguish it from the dispatcher options.
Is it ready for merging, or does it need work?
Feedback is wanted but it's merge ready from my end.
closes #6357