Skip to content

Reduce tokenizer/parser overhead#4

Closed
zao111222333 wants to merge 1 commit intogwenn:highlightfrom
zao111222333:mutable_helper
Closed

Reduce tokenizer/parser overhead#4
zao111222333 wants to merge 1 commit intogwenn:highlightfrom
zao111222333:mutable_helper

Conversation

@zao111222333
Copy link
Copy Markdown

@zao111222333 zao111222333 commented Oct 4, 2024

Make helper mutable in highlight, validate, complete, ...
as you have mentioned at
https://github.com/gwenn/rustyline/blob/highlight/src/completion.rs#L93
https://github.com/gwenn/rustyline/blob/highlight/src/lib.rs#L543
Now the Helper becomes to

rustyline/src/lib.rs

Lines 544 to 573 in 6e08e81

/// Syntax specific helper.
///
/// TODO Tokenizer/parser used for both completion, suggestion, highlighting.
/// (parse current line once)
pub trait Helper
where
Self: Completer + Hinter + Highlighter + Validator,
{
/// Update helper when line has been modified.
///
/// This is the first-called function just after the editing is done,
/// before all other functions within [Completer], [Hinter], [Highlighter], and [Validator].
///
/// You can put the tokenizer/parser here so that other APIs can directly use
/// results generate here, and reduce the overhead.
fn update_after_edit(&mut self, line: &str, pos: usize, forced_refresh: bool) {
_ = (line, forced_refresh, pos);
}
/// Update helper when cursor has been moved.
///
/// This is the first-called function just after the cursor moving is done,
/// before all other functions within [Completer], [Hinter], [Highlighter], and [Validator].
///
/// You can put the tokenizer/parser here so that other APIs can directly use
/// results generate here, and reduce the overhead.
fn update_after_move_cursor(&mut self, line: &str, pos: usize) {
_ = (line, pos);
}
}

And APIs like complete become to
pub trait Completer {
/// Specific completion candidate.
type Candidate: Candidate;
// TODO: let the implementers choose/find word boundaries ??? => Lexer
/// Takes the currently edited `line` with the cursor `pos`ition and
/// returns the start position and the completion candidates for the
/// partial word to be completed.
///
/// ("ls /usr/loc", 11) => Ok((3, vec!["/usr/local/"]))
fn complete(
&mut self, // FIXME should be `&mut self`
line: &str,
pos: usize,
ctx: &Context<'_>,
) -> Result<(usize, Vec<Self::Candidate>)> {
let _ = (line, pos, ctx);
Ok((0, Vec::with_capacity(0)))
}
/// Updates the edited `line` with the `elected` candidate.
fn update(&mut self, line: &mut LineBuffer, start: usize, elected: &str, cl: &mut Changeset) {
let end = line.pos();
line.replace(start..end, elected, cl);
}
}

User can put the tokenizer/parser at update_after_edit, which will be called just after the line is modified, to update context. Then use the parsed context in highlight, validate, complete, ...

The examples/continuation_prompt.rs is just a prototype to demo this featrue, and will be updated during the continuation_prompt with new layout.

/// ("ls /usr/loc", 11) => Ok((3, vec!["/usr/local/"]))
fn complete(
&self, // FIXME should be `&mut self`
&mut self, // FIXME should be `&mut self`
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIXME not removed

)*
}
}
// macro_rules! box_completer {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure ?

let width = candidate.width();
if let Some(highlighter) = s.highlighter() {
ab.push_str(&highlighter.highlight_candidate(candidate, CompletionType::List));
if s.out.colors_enabled() {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong: duplicated code (previously this logic was factorized in s.highlighter() method).

///
/// You can put the tokenizer/parser here so that other APIs can directly use
/// results generate here, and reduce the overhead.
fn update_after_edit(&mut self, line: &str, pos: usize, forced_refresh: bool) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have prefer a dedicated component and an API compatible with tree-sitter incremental parsing.
See https://github.com/gwenn/rustyline-notes/blob/master/src/syntax.md

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this may conflict with another feature: autocorrection.
See kkawakam#593 (comment)
And kkawakam#656 (comment)

///
/// You can put the tokenizer/parser here so that other APIs can directly use
/// results generate here, and reduce the overhead.
fn update_after_move_cursor(&mut self, line: &str, pos: usize) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems useless to me.

@gwenn
Copy link
Copy Markdown
Owner

gwenn commented Oct 4, 2024

If possible, I would prefer a dedicated PR for mutable helper based on rustyline master branch instead of the highlight branch.
See also kkawakam@023a080
(Option<&'out mut H> vs &'out mut Option<H>)

@zao111222333
Copy link
Copy Markdown
Author

zao111222333 commented Oct 4, 2024

Thx for inform me https://github.com/gwenn/rustyline-notes/blob/master/src/syntax.md, which is exactly what I want. But for Parser::Document or Helper::Document, I prefer use Helper itself as "Document", that is, remove the Helper::Document and update all parsed things into Helper itself.
Otherwise, as you mentioned

How to make sure all traits are coherent/have the same Document type ?

This requirement will make traits messy. Like

pub trait Helper
where
    Self: Completer<Parsed = <Self as Helper>::Parsed>
        + Hinter<Parsed = <Self as Helper>::Parsed>
        + Highlighter<Parsed = <Self as Helper>::Parsed>
        + Parser<Parsed = <Self as Helper>::Parsed>
        + Validator<Parsed = <Self as Helper>::Parsed>,
{
    type Parsed;
}

pub trait Completer {
    type Parsed;
}
...

And also, thanks a lot for reviewing. I will make a commit directly to master branch, under your suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants