Hi just a small bug, I think you meant to use strip_suffix for >, here?
Activity
13Yes sure.
But to be honest, the main issue that made me read the parse_key_sequence fn is that as far as I understand it, it cannot parse a < or > key. In some layout like mine (italian) < is a main key in place of \, so it would be quite useful to be able to set it.
I just came up with this regex based alternative fn, what do you think? You can then do "<\\>>": "Quit" to escape any key. I can polish and test it then make a PR for this instead, maybe?
use regex::Regex;
pub fn parse_key_sequence(raw: &str) -> Result<Vec<KeyEvent>, String> {
let key_event_re = Regex::new(r"<(?<keyevent>(?:[^<>\\]|\\.)*)>").unwrap();
let no_backslash_re = Regex::new(r"\\(.)").unwrap();
key_event_re.captures_iter(raw)
.map(|c| {
let (_, [key_event_string]) = c.extract();
key_event_string
})
.map(|s|
{
let no_backslash: &str = &*no_backslash_re.replace_all(s, "$1").into_owned();
parse_key_event(no_backslash)
})
.collect()
}Yea no ofc need to be cleaned up a bit, no need for 2 regexes probably, it was just to show the idea more clearly.
Otoh I hope am not missing anything important on the technical details of keyboards and character classes, I'm not super knowledgeable about them.
Nice find! This is what I get for not writing comprehensive tests :) I'm fine with a regex fix for this with more tests.
Also, certainly check out https://github.com/Canop/crokey/ for a more robust key parsing. I thought @joshka had something too but can't find it on his GitHub.
I'd suggest to add to this:
- Use verbose mode (ignore whitespace, allow newlines and end of line comments) to make the regex easier to read
- Compile the regex for perf
- Add tests and docs (this is complex and it's difficult to figure out the exact intent behind this and the expected edge cases)
- I think this could be better (simpler config, easier code) without the angle brackets. I.e. just use a comma to separate keys.
- Consider moving the regex up one level in the call stack and handle single key strokes as a single item sequence.
I thought @joshka had something too but can't find it on his GitHub.
You're talking about https://crates.io/crates/keybinding. It's still a placeholder. Also read https://github.com/ratatui-org/ratatui/discussions/627 for context around this.
Is there any particular reason crokey isn't used for the parsing here?
Just personal preference when I wrote the template and I pulled code from an existing project and refactored it to add it to the template. I think crokey would be the way to go.
Alright, then no need to reinvent the wheel with regex, prob better to go that way directly.
I'm a bit busy right now, but I'll try to see how much work porting the settings to crokey would be.
Hey @f-forcher, kindly ping to check if you are still interested in going forward with this 🐻
Hi guys, sorry I didn't forgot, but I have been interviewing full time since then 😓 I had to put personal projects on the backlog a bit. Now I have a bit of a lull next week l, I'll try to see to it
Any updates on this? 👀
Hi, well I did forget in the end as I paused projects. I'll try to have a look at it again
Yup, do you mind sending a PR to fix it? 🐻