Strip prefix/suffix

[f-forcher]

Hi just a small bug, I think you meant to use strip_suffix for >, here?

templates/component/template/src/config.rs

Lines 251 to 252 in 966cf2e

	let raw = raw.strip_prefix('<').unwrap_or(raw);
	let raw = raw.strip_prefix('>').unwrap_or(raw);

[orhun]

Yup, do you mind sending a PR to fix it? 🐻

[f-forcher]

Yes 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()
}

[orhun]

I'm not sure if the code can be simplified a bit but improving this sounds good to me. Any thoughts @kdheepak @joshka ?

[f-forcher]

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.

[kdheepak]

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.

[joshka]

I'd suggest to add to this:

1. Use verbose mode (ignore whitespace, allow newlines and end of line comments) to make the regex easier to read
2. Compile the regex for perf
3. Add tests and docs (this is complex and it's difficult to figure out the exact intent behind this and the expected edge cases)
4. I think this could be better (simpler config, easier code) without the angle brackets. I.e. just use a comma to separate keys.
5. 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 ratatui-org/ratatui#627 for context around this.

[f-forcher]

Is there any particular reason crokey isn't used for the parsing here?

[kdheepak]

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.

[f-forcher]

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.

[orhun]

Hey @f-forcher, kindly ping to check if you are still interested in going forward with this 🐻

[f-forcher]

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