Skip to content
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

make the completer return the strings (or positions of them) that are used for completing #711

Closed
maxomatic458 opened this issue Jan 21, 2024 · 9 comments · Fixed by #713
Labels
enhancement New feature or request

Comments

@maxomatic458
Copy link
Contributor

I think this the complete method could be changed, so it will also return the strings that were used to generate the Suggestions.

something like this:

fn complete(&mut self, line: &str, pos: usize) -> (Vec<Suggestion>, Vec<String>);

for example:
buffer: "this is t"
calling .complete could return the suggestions ("this is the reedline crate", "test", ...) alongside the strings that were used to calculate the suggestions (in this case "this is t", "t")

this info would be useful for menus (for example for greying out text already typed), or in the case of the new ide menu, change the position so it lines up with the typed text (instead of just the cursor)

@maxomatic458 maxomatic458 added the enhancement New feature or request label Jan 21, 2024
@fdncred
Copy link
Collaborator

fdncred commented Jan 21, 2024

Seems plausible but sounds like it would be a breaking api change. Not that we're against that but seems like it may be tricky to retrofit in nushell. Maybe I'm wrong?

@maxomatic458
Copy link
Contributor Author

maxomatic458 commented Jan 21, 2024

i guess we could make a seperate function for this then, but i feel like this just makes things unnecessarily complicated

But if we would do that, then there wont be any breaking changes

Edit: actually i think having a seperate function would probably be the better way to go

@fdncred
Copy link
Collaborator

fdncred commented Jan 21, 2024

Separate function sounds great. If you put a //TODO in there around the function, to sometime consolidate functions, we can decide at a later point how important it is and if it's worth a breaking change. Thanks for bringing up this topic.

@maxomatic458
Copy link
Contributor Author

to make this function "optional" then
should i have a default implementation that panics or returns default values, when its not explicitly implemented?

something like this

pub trait Completer: Send {
    /// the action that will take the line and position and convert it to a vector of completions, which include the
    /// span to replace and the contents of that replacement
    fn complete(&mut self, line: &str, pos: usize) -> Vec<Suggestion>;
    
    /// complete, but also return the index of the start of the string being completed
    fn complete_with_start(&mut self, line: &str, pos: usize) -> (Vec<Suggestion>, usize) {
        panic!("Not implemented")
    }
    ...
}

@fdncred
Copy link
Collaborator

fdncred commented Jan 21, 2024

Is it possible to have complete_with_start call complete if complete_with_start isn't implemented? Kind of uses the default implementation of complete if complete_with_start isn't implemented.

@maxomatic458
Copy link
Contributor Author

@fdncred that would only work if we can assume that the suggestions starts with the last part of the line. then the start position can be calculated from that. otherwise (if the completer uses a different way, like levensthein distance of the last word or so)
then this will return a wrong value

but you could also return a default position like 0, but i think it would be better to panic then returning something potentially wrong

@maxomatic458
Copy link
Contributor Author

all of the current menus that support this can just do this

impl Completer for DefaultCompleter {
    fn complete(&mut self, line: &str, pos: usize) -> Vec<Suggestion> {
        complete_with_start(&mut self, line: &str, pos: usize).0
    }
    
    fn complete_with_start(&mut self, line: &str, pos: usize) -> (Vec<Suggestion>, usize) {
        ...
    }
    ...
}

@fdncred
Copy link
Collaborator

fdncred commented Jan 21, 2024

I'm up for whatever. BTW - I've never really liked Levenstein completions. I always thought alphabetical was better.

@maxomatic458
Copy link
Contributor Author

turns out each suggestion already includes the range in question.
so a base implementation does the trick

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants