Make implementing Helper easier

[HarrisonMc555]

I am just starting to look at rustyline, and it looks good so far. However, I decided that I might want to try to implement a simple Hinter.

As far as I understand it, every Editor has an associated Helper type. This can be (), which has default (empty) implementations of the required Completer, Hinter, and Highlighter traits. However, if you want to implement your own Helper, that type needs to implement all three of these traits. Doing so requires a lot of boilerplate code if you want "no-op" implementations of certain traits.

Is there any way we could ease this and make it so that if you only wanted to implement one of these, you could do so? If I understand correctly, I almost think you could move the current implementations for () into the trait definitions to give them default implementations. I'm not 100% sure about associated types (i.e. Completer), but the rest should be fine.

[gwenn]

I don't know: https://github.com/kkawakam/rustyline/issues/197#issuecomment-458293088 Maybe introduce some features ?

[HarrisonMc555]

Oh that makes it a lot easier. The only one that's not quite as easy is the Completer. I think we would still have to declare the associated type (String is what I did since it has a pre-defined implementation), but I believe the required methods for Completer could have default no-op implementations.

[oppiliappan]

A Helper shouldn't have to have all three trait implementations.
Maybe we can have something like set_hinter, set_highlighter and set_completer?

Right now, its a little roundabout to implement just one of the three, especially if I want to skip the Completer

[gwenn]

For one syntax, Hinter, Highlighter and Completer, Validator should/would/will be correlated. Because they should/would/will share the same Tokenizer/Parser. Currently, each one parses the same input... See https://github.com/kkawakam/rustyline/issues/197#issuecomment-458293088

I guess that this cannot be done if we introduce set_hinter, ...

[gwenn]

See #267

[thomcc]

Maybe I'm missing something, but why are these all separate traits instead of just one trait with defaulted functions? It seems like the separation just means that each additional feature is a semver-breaking change, and it adds boilerplate to implementers.

[gwenn]

@thomcc Because you may want to reuse the FilenameCompleter or the HistoryHinter but implement your own highlighter. Or someone may implement a reusable highlighter based on syntect/tree-sitter by specifying only the language extension...

[thomcc]

Yeah, so I don't really see why those are incompatible. In fact, the way it is now makes it more annoying to reuse, since if even when you do reuse those things, you need to forward stuff along.

Here's one option:

pub trait Helper {
    fn hinter(&self) -> Option<&dyn Hinter> { None }
    fn highlighter(&self) -> Option<&dyn Highlighter> { None }
    fn validator(&self) -> Option<&dyn Validator> { None }
    fn completer(&self) -> Option<&dyn Completer> { None }
}

Then to reuse things, you would just return Some(&self.hinter) rather than having to implement several methods that forward the arguments to self.hinter.

(The wrinkle here is that this works for all of these except Completer, but I think that can be made to work too).

There are other possible designs, but the nice associated type one that comes to mind doesn't actually work, due to associated type defaults not being a thing yet.

The benefit here is that you get an easier to implement Helper type, and you don't have to bump major versions every time you add a new feature which would require something new of Helper.

[gwenn]

@thomcc Yes, your solution should work. We will need something like that:

pub trait Helper {
    type Document;

    fn lexer(&self) -> Option<&dyn Lexer<Self::Document>> { None }
    fn hinter(&self) -> Option<&dyn Hinter<Self::Document>> { None }
    fn highlighter(&self) -> Option<&dyn Highlighter<Self::Document>> { None }
    fn validator(&self) -> Option<&dyn Validator<Self::Document>> { None }
    fn completer(&self) -> Option<&dyn Completer<Self::Document>> { None }
}

pub trait Lexer {
    type Document;

    fn lex(&self) -> Self::Document;    
}

pub trait Completer {
    type Candidate: Candidate;
    type Document;

    fn complete(&self, doc: Self::Document, ...) -> Result<(usize, Vec<Self::Candidate>)> ...
}
...

[thomcc]

Hmm, I see... I saw you refer to this in the BUGS doc, and this refers to a similar architecture to https://python-prompt-toolkit.readthedocs.io/en/master/pages/advanced_topics/rendering_flow.html#the-rendering-flow ?

1. I think the optimization that allows for incremental parsing is probably pointless -- These input buffers are almost always small. THe bottlenecks has definitely been writing stuff to the terminal from what I've seen. (And semantic analysis -- but nothing rustyline does will speed that up)
   • That said, there are definitely parts of their stuff that seem like they'd help here -- buffering, change tracking, etc.
2. I really like that they manage styling -- IMO rustyline should do that too, rather than have you write escapes and hope for the best.
3. The other thing I like is that each step (each Processor) seems to be able to update a lot about the output things -- When working on evcxr's repl, I found it frustrating that I couldn't modify the styling info from inside the validator.

I'll think about it. I suspect that you probably don't need to make document fully generic -- I can always build it internally if I need it everywhere. I'll think about it though.

[gwenn]

@thomcc Only the highlighter should modify the styling because the terminal or the user doesn't want ANSI code...