Skip to content

Conversation

@blindFS
Copy link
Contributor

@blindFS blindFS commented Oct 25, 2025

Move the partial completion logic from the handling of ReedlineEvent::MenuNext, so that we can fix nushell/nushell#14152 while keeping the following behavior.

When the only valid options are say foo and foooo, a typical bash behavior of f#tab would complete the repl text to foo without moving the menu item towards foooo.

Closes #881

Comment on lines -508 to -513
// If the values were already updated (e.g. quick completions are true)
// there is no need to update the values from the menu
if !values_updated {
self.update_values(editor, completer);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not very sure that it's safe to remove this. But I think it's a bad idea to handle quick completion related menu updates here.

@blindFS blindFS marked this pull request as ready for review October 25, 2025 10:40
@fdncred
Copy link
Contributor

fdncred commented Oct 25, 2025

Are there tests we can add for this plus the quick complete section that was removed?

@blindFS
Copy link
Contributor Author

blindFS commented Oct 25, 2025

Are there tests we can add for this plus the quick complete section that was removed?

That's a good question.

  1. I can't find any unit test for reedline events, seems that it is the function behind each event handler that's being tested.
  2. As for the removed part, I've changed this macro (same for ide_menu.rs) so that existing tests for partial completions shall pass. But I don't think that's good enough. The fact that we have to call menu.update_values twice (one before the can_partial_complete check, one inside it) for partial_completion tasks bugs me. I don't know if I can help to optimize it, but that's probably off-topic.

macro_rules! partial_completion_tests {
(name: $test_group_name:ident, completions: $completions:expr, test_cases: $($name:ident: $value:expr,)*) => {
mod $test_group_name {
use crate::{menu::Menu, ColumnarMenu, core_editor::Editor, enums::UndoBehavior};
use super::FakeCompleter;
$(
#[test]
fn $name() {
let (input, expected) = $value;
let mut menu = ColumnarMenu::default();
let mut editor = Editor::default();
editor.set_buffer(input.to_string(), UndoBehavior::CreateUndoPoint);
let mut completer = FakeCompleter::new(&$completions);
menu.can_partially_complete(false, &mut editor, &mut completer);
assert_eq!(editor.get_buffer(), expected);
}
)*
}
}
}

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.

tab completion on suggestions stuck on the second element

2 participants