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

made can_partially_complete() only complete when suggesting more than the input #834

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

uek-1
Copy link
Contributor

@uek-1 uek-1 commented Sep 26, 2024

Addresses #833 . menu.can_partially_complete() would reset the current selection 0 and return true even if the suggested partial completion was exactly the same. Changed can_partially_complete in menu_functions to return false and not complete if the suggested partial completion is the exact same as the input.

@fdncred
Copy link
Collaborator

fdncred commented Sep 26, 2024

I'm trying to test this but can't get it to work. Here's what I did.

cargo run --example demo
a<tab><tab><tab>

I can't get any of the tabs to progress. I figure I'm doing something wrong. Please let me know.

Strangely enough, shift-tab will go backwards through all the items, but tab won't go forwards through them.

@uek-1
Copy link
Contributor Author

uek-1 commented Sep 27, 2024

I think this might be an issue with the demo itself. Tabs don't seem to progress as of d8b240a either. Looking at the code, I see that MenuNext is actually not bound to tab in the demo (binding it does fix issue).

reedline/examples/demo.rs

Lines 244 to 250 in 660a507

keybindings.add_binding(
KeyModifiers::NONE,
KeyCode::Tab,
ReedlineEvent::UntilFound(vec![
ReedlineEvent::Menu("completion_menu".to_string()),
ReedlineEvent::Edit(vec![EditCommand::Complete]),
]),

@fdncred
Copy link
Collaborator

fdncred commented Sep 27, 2024

Can you submit a PR for that fix to the demo.rs app too please, in a separate PR? This makes total sense.

@uek-1 uek-1 mentioned this pull request Sep 27, 2024
@132ikl
Copy link

132ikl commented Oct 6, 2024

Is there anything blocking this PR?

@fdncred
Copy link
Collaborator

fdncred commented Oct 7, 2024

nope. i was hoping to get more eyes on it but doesn't look like anyone is objecting.

@fdncred fdncred merged commit 871075e into nushell:main Oct 7, 2024
6 checks passed
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.

3 participants