-
Notifications
You must be signed in to change notification settings - Fork 36
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
Strip prefix/suffix #52
Comments
Yup, do you mind sending a PR to fix it? 🐻 |
Yes sure. But to be honest, the main issue that made me read the I just came up with this regex based alternative fn, what do you think? You can then do 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:
You're talking about https://crates.io/crates/keybinding. It's still a placeholder. Also read ratatui/ratatui#627 for context around this. |
Is there any particular reason |
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 |
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 |
Hi just a small bug, I think you meant to use
strip_suffix
for>
, here?https://github.com/ratatui-org/templates/blob/966cf2e2b5808de8c905eacd1f4209fe82f804fe/component/template/src/config.rs#L251-L252
The text was updated successfully, but these errors were encountered: