kkawakam / rustyline

Readline Implementation in Rust
https://crates.io/crates/rustyline/
MIT License
1.53k stars 176 forks source link

Refactor highlighting #795

Open gwenn opened 4 weeks ago

gwenn commented 4 weeks ago

Introduce Style and StyledBlock traits And their ansi-str implementations

See #793, #372 See https://github.com/gwenn/rustyline-notes/blob/master/src/design.md

zao111222333 commented 3 weeks ago

It seems that we can change the ansi_str::get_blocks form fn get_blocks(text: &str) to fn get_blocks<'s>(text: Cow<'s, str>). And then all those error here 6b34d32 will be solved. (In this case we have to create a new crate rather than use ansi_str since I don't know the origin author's opinion with this change). Shall I have a try on this approach and make a commit?

zao111222333 commented 3 weeks ago

I find that once we define

fn highlight_line<'l>(&self, line: &'l str, pos: usize) -> impl Iterator<Item = impl 'l+StyledBlock>

The new highlight_line function signature conflicts to the Option<&dyn Highligher> due to https://doc.rust-lang.org/reference/items/traits.html#object-safety . So use generic <H: Highligher> + Option<&H> to replace Option<&dyn Highligher>

And I have maked a pull request, see https://github.com/kkawakam/rustyline/pull/799

gwenn commented 3 weeks ago

Maybe we should not try to implement highlight_line for ansi_str. See https://github.com/kkawakam/rustyline/pull/795/commits/92bcae0ae39839f0045dbc4e9f3e0971f3857d8b

zao111222333 commented 2 weeks ago

Looks good! And how do you think Vec<(Self::Style, &'l str)> or impl Iterator<(Self::Style, &'l str)>? The cons and pros of impl Iterator<(Self::Style, &'l str)>: pros:

cons:

gwenn commented 2 weeks ago

Alternatives use Vec: https://docs.rs/syntect/5.2.0/syntect/easy/struct.HighlightLines.html#method.highlight https://docs.rs/reedline/latest/reedline/trait.Highlighter.html#tymethod.highlight + https://docs.rs/reedline/latest/reedline/struct.StyledText.html https://docs.rs/termwiz/latest/termwiz/lineedit/trait.LineEditorHost.html#method.highlight_line

gwenn commented 2 weeks ago

Highlighter implementations are becoming messy. See 9e5c0e1 Maybe we should remove the type Style associated type and support only () or anstyle::Style ? Knowing that we also need to fix highlight_hint...

zao111222333 commented 2 weeks ago

Thanks a lot for your hard working, I believe impl Iterator<Item = impl 'l + StyledBlock> is the best function signature. And what can I do for this feature?

gwenn commented 2 weeks ago

And what can I do for this feature?

See above: [ ] Fix rendering accordingly [ ] Check that new API works with an hardcoded continuation prompt.