-
-
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
Allow a custom delay before a command popup shows up #2715
Conversation
826872e
to
b818b76
Compare
a1a1585
to
53432ec
Compare
There's an interesting edge case with regards to the Other than that, this works really well, nice job! |
I wonder if it wouldn't be unjust to make a default value of 600 -- in my testing it feels like about the delay i would have when unsure of the keybind i want, while not showing up for times when im a little slower to press the next binding when i do know what it is. |
There's a PR reworking |
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.
Places where we call cx.on_next_key
don't get the effect of the delay, like select_textobject
in commands.rs
. We may want to introduce a cx.on_next_key_with_autoinfo
like Kakoune has so we can close over adding fake autoinfo with delay nicely https://github.com/mawww/kakoune/blob/689553c2e9b953a9d3822528d4ad858af95fb6a2/src/input_handler.hh#L190
} | ||
}), | ||
// delay cancelled | ||
_ = cancel_rx => Box::new(move |_editor, _compositor| {}), |
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.
Ah this is clever - this doesn't need to be triggered explicitly because it's marked as completed in the Drop implementation for Sender, eh?
It's probably worth a comment here on how this triggers since it's kinda hidden in the implementation details
let is_pending = matches!(key_result, KeymapResult::Pending(_)); | ||
if !is_pending { |
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.
I don't think it would be too unreadable to have the matches!
in the if
expression here. I don't feel strongly about it though and I think you did this for readability so it's up to you if you want to keep it as-is
Any chance to add in the case of 0 (ms), then the popup is never shown? Even better per command so it could be customized in config.toml? |
You can disable the auto-info boxes entirely already with the # ~/.config/helix/config.toml
[editor]
auto-info = false |
Is this PR abandoned? I love this feature |
Agreed, I'd love it too. I've become addicted to using this editor. I wish my Rust was more advanced so I could contribute. Meantime I can only echo that this feature would solve the one thing that constantly annoys me. |
I would really love to see this merged soon! |
closing this one out as stale. Thank you for contributing! |
Hi!
This pull request closes #1475.