fix: Recalculate completion when going through prompt history (#3193)

* fix: Recalculate completion when going through prompt history

* Update completion when the prompt line is changed

It should not be possible to update the line without also updating the
completion since the completion holds an index into the line.

* Fix Prompt::with_line recalculate completion

with_line was the last function where recalculate completion had to be
done manually. This function now also recalculates the completion so
that it's impossible to forget.

* Exit selection when recalculating completion

Keeping the selection index when the completion has been recalculated
doesn't make sense. This clears the selection automatically, removing
most needs to manually clear it.

* Remove &mut on save_filter

Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>

helix-term/src/commands/dap.rs

@@ -582,7 +582,7 @@ pub fn dap_edit_condition(cx: &mut Context) {
             None => return,
         };
         let callback = Box::pin(async move {
-            let call: Callback = Box::new(move |_editor, compositor| {
+            let call: Callback = Box::new(move |editor, compositor| {
                 let mut prompt = Prompt::new(
                     "condition:".into(),
                     None,
@@ -607,7 +607,7 @@ pub fn dap_edit_condition(cx: &mut Context) {
                     },
                 );
                 if let Some(condition) = breakpoint.condition {
-                    prompt.insert_str(&condition)
+                    prompt.insert_str(&condition, editor)
                 }
                 compositor.push(Box::new(prompt));
             });
@@ -624,7 +624,7 @@ pub fn dap_edit_log(cx: &mut Context) {
             None => return,
         };
         let callback = Box::pin(async move {
-            let call: Callback = Box::new(move |_editor, compositor| {
+            let call: Callback = Box::new(move |editor, compositor| {
                 let mut prompt = Prompt::new(
                     "log-message:".into(),
                     None,
@@ -648,7 +648,7 @@ pub fn dap_edit_log(cx: &mut Context) {
                     },
                 );
                 if let Some(log_message) = breakpoint.log_message {
-                    prompt.insert_str(&log_message);
+                    prompt.insert_str(&log_message, editor);
                 }
                 compositor.push(Box::new(prompt));
             });

helix-term/src/ui/mod.rs

@@ -35,10 +35,10 @@ pub fn prompt(
     completion_fn: impl FnMut(&Editor, &str) -> Vec<prompt::Completion> + 'static,
     callback_fn: impl FnMut(&mut crate::compositor::Context, &str, PromptEvent) + 'static,
 ) {
-    show_prompt(
+    let mut prompt = Prompt::new(prompt, history_register, completion_fn, callback_fn);
-        cx,
+    // Calculate the initial completion
-        Prompt::new(prompt, history_register, completion_fn, callback_fn),
+    prompt.recalculate_completion(cx.editor);
-    );
+    cx.push_layer(Box::new(prompt));
 }
 
 pub fn prompt_with_input(
@@ -49,15 +49,8 @@ pub fn prompt_with_input(
     completion_fn: impl FnMut(&Editor, &str) -> Vec<prompt::Completion> + 'static,
     callback_fn: impl FnMut(&mut crate::compositor::Context, &str, PromptEvent) + 'static,
 ) {
-    show_prompt(
+    let prompt = Prompt::new(prompt, history_register, completion_fn, callback_fn)
-        cx,
+        .with_line(input, cx.editor);
-        Prompt::new(prompt, history_register, completion_fn, callback_fn).with_line(input),
-    );
-}
-
-fn show_prompt(cx: &mut crate::commands::Context, mut prompt: Prompt) {
-    // Calculate initial completion
-    prompt.recalculate_completion(cx.editor);
     cx.push_layer(Box::new(prompt));
 }
 

helix-term/src/ui/picker.rs

@@ -470,7 +470,7 @@ impl<T: Item> Picker<T> {
         self.filters
             .extend(self.matches.iter().map(|(index, _)| *index));
         self.filters.sort_unstable(); // used for binary search later
-        self.prompt.clear(cx);
+        self.prompt.clear(cx.editor);
     }
 
     pub fn toggle_preview(&mut self) {

helix-term/src/ui/prompt.rs

@@ -83,10 +83,11 @@ impl Prompt {
         }
     }
 
-    pub fn with_line(mut self, line: String) -> Self {
+    pub fn with_line(mut self, line: String, editor: &Editor) -> Self {
         let cursor = line.len();
         self.line = line;
         self.cursor = cursor;
+        self.recalculate_completion(editor);
         self
     }
 
@@ -95,6 +96,7 @@ impl Prompt {
     }
 
     pub fn recalculate_completion(&mut self, editor: &Editor) {
+        self.exit_selection();
         self.completion = (self.completion_fn)(editor, &self.line);
     }
 
@@ -213,12 +215,12 @@ impl Prompt {
             self.cursor = pos;
         }
         self.recalculate_completion(cx.editor);
-        self.exit_selection();
     }
 
-    pub fn insert_str(&mut self, s: &str) {
+    pub fn insert_str(&mut self, s: &str, editor: &Editor) {
         self.line.insert_str(self.cursor, s);
         self.cursor += s.len();
+        self.recalculate_completion(editor);
     }
 
     pub fn move_cursor(&mut self, movement: Movement) {
@@ -234,65 +236,65 @@ impl Prompt {
         self.cursor = self.line.len();
     }
 
-    pub fn delete_char_backwards(&mut self, cx: &Context) {
+    pub fn delete_char_backwards(&mut self, editor: &Editor) {
         let pos = self.eval_movement(Movement::BackwardChar(1));
         self.line.replace_range(pos..self.cursor, "");
         self.cursor = pos;
 
-        self.exit_selection();
+        self.recalculate_completion(editor);
-        self.recalculate_completion(cx.editor);
     }
 
-    pub fn delete_char_forwards(&mut self, cx: &Context) {
+    pub fn delete_char_forwards(&mut self, editor: &Editor) {
         let pos = self.eval_movement(Movement::ForwardChar(1));
         self.line.replace_range(self.cursor..pos, "");
 
-        self.exit_selection();
+        self.recalculate_completion(editor);
-        self.recalculate_completion(cx.editor);
     }
 
-    pub fn delete_word_backwards(&mut self, cx: &Context) {
+    pub fn delete_word_backwards(&mut self, editor: &Editor) {
         let pos = self.eval_movement(Movement::BackwardWord(1));
         self.line.replace_range(pos..self.cursor, "");
         self.cursor = pos;
 
-        self.exit_selection();
+        self.recalculate_completion(editor);
-        self.recalculate_completion(cx.editor);
     }
 
-    pub fn delete_word_forwards(&mut self, cx: &Context) {
+    pub fn delete_word_forwards(&mut self, editor: &Editor) {
         let pos = self.eval_movement(Movement::ForwardWord(1));
         self.line.replace_range(self.cursor..pos, "");
 
-        self.exit_selection();
+        self.recalculate_completion(editor);
-        self.recalculate_completion(cx.editor);
     }
 
-    pub fn kill_to_start_of_line(&mut self, cx: &Context) {
+    pub fn kill_to_start_of_line(&mut self, editor: &Editor) {
         let pos = self.eval_movement(Movement::StartOfLine);
         self.line.replace_range(pos..self.cursor, "");
         self.cursor = pos;
 
-        self.exit_selection();
+        self.recalculate_completion(editor);
-        self.recalculate_completion(cx.editor);
     }
 
-    pub fn kill_to_end_of_line(&mut self, cx: &Context) {
+    pub fn kill_to_end_of_line(&mut self, editor: &Editor) {
         let pos = self.eval_movement(Movement::EndOfLine);
         self.line.replace_range(self.cursor..pos, "");
 
-        self.exit_selection();
+        self.recalculate_completion(editor);
-        self.recalculate_completion(cx.editor);
     }
 
-    pub fn clear(&mut self, cx: &Context) {
+    pub fn clear(&mut self, editor: &Editor) {
         self.line.clear();
         self.cursor = 0;
-        self.recalculate_completion(cx.editor);
+        self.recalculate_completion(editor);
-        self.exit_selection();
     }
 
-    pub fn change_history(&mut self, register: &[String], direction: CompletionDirection) {
+    pub fn change_history(
+        &mut self,
+        cx: &mut Context,
+        register: char,
+        direction: CompletionDirection,
+    ) {
+        let register = cx.editor.registers.get_mut(register).read();
+
         if register.is_empty() {
             return;
         }
@@ -312,6 +314,7 @@ impl Prompt {
         self.history_pos = Some(index);
 
         self.move_end();
+        self.recalculate_completion(cx.editor);
     }
 
     pub fn change_completion_selection(&mut self, direction: CompletionDirection) {
@@ -494,16 +497,18 @@ impl Component for Prompt {
             ctrl!('f') | key!(Right) => self.move_cursor(Movement::ForwardChar(1)),
             ctrl!('e') | key!(End) => self.move_end(),
             ctrl!('a') | key!(Home) => self.move_start(),
-            ctrl!('w') | alt!(Backspace) | ctrl!(Backspace) => self.delete_word_backwards(cx),
+            ctrl!('w') | alt!(Backspace) | ctrl!(Backspace) => {
-            alt!('d') | alt!(Delete) | ctrl!(Delete) => self.delete_word_forwards(cx),
+                self.delete_word_backwards(cx.editor)
-            ctrl!('k') => self.kill_to_end_of_line(cx),
+            }
-            ctrl!('u') => self.kill_to_start_of_line(cx),
+            alt!('d') | alt!(Delete) | ctrl!(Delete) => self.delete_word_forwards(cx.editor),
+            ctrl!('k') => self.kill_to_end_of_line(cx.editor),
+            ctrl!('u') => self.kill_to_start_of_line(cx.editor),
             ctrl!('h') | key!(Backspace) => {
-                self.delete_char_backwards(cx);
+                self.delete_char_backwards(cx.editor);
                 (self.callback_fn)(cx, &self.line, PromptEvent::Update);
             }
             ctrl!('d') | key!(Delete) => {
-                self.delete_char_forwards(cx);
+                self.delete_char_forwards(cx.editor);
                 (self.callback_fn)(cx, &self.line, PromptEvent::Update);
             }
             ctrl!('s') => {
@@ -520,14 +525,13 @@ impl Component for Prompt {
                 );
                 let line = text.slice(range.from()..range.to()).to_string();
                 if !line.is_empty() {
-                    self.insert_str(line.as_str());
+                    self.insert_str(line.as_str(), cx.editor);
                     (self.callback_fn)(cx, &self.line, PromptEvent::Update);
                 }
             }
             key!(Enter) => {
                 if self.selection.is_some() && self.line.ends_with(std::path::MAIN_SEPARATOR) {
                     self.recalculate_completion(cx.editor);
-                    self.exit_selection();
                 } else {
                     // handle executing with last command in history if nothing entered
                     let input: Cow<str> = if self.line.is_empty() {
@@ -553,15 +557,13 @@ impl Component for Prompt {
             }
             ctrl!('p') | key!(Up) => {
                 if let Some(register) = self.history_register {
-                    let register = cx.editor.registers.get_mut(register);
+                    self.change_history(cx, register, CompletionDirection::Backward);
-                    self.change_history(register.read(), CompletionDirection::Backward);
                     (self.callback_fn)(cx, &self.line, PromptEvent::Update);
                 }
             }
             ctrl!('n') | key!(Down) => {
                 if let Some(register) = self.history_register {
-                    let register = cx.editor.registers.get_mut(register);
+                    self.change_history(cx, register, CompletionDirection::Forward);
-                    self.change_history(register.read(), CompletionDirection::Forward);
                     (self.callback_fn)(cx, &self.line, PromptEvent::Update);
                 }
             }
@@ -570,7 +572,6 @@ impl Component for Prompt {
                 // if single completion candidate is a directory list content in completion
                 if self.completion.len() == 1 && self.line.ends_with(std::path::MAIN_SEPARATOR) {
                     self.recalculate_completion(cx.editor);
-                    self.exit_selection();
                 }
                 (self.callback_fn)(cx, &self.line, PromptEvent::Update)
             }
@@ -602,8 +603,8 @@ impl Component for Prompt {
                             .read(c)
                             .and_then(|r| r.first())
                             .map_or("", |r| r.as_str()),
+                        context.editor,
                     );
-                    prompt.recalculate_completion(context.editor);
                 }));
                 (self.callback_fn)(cx, &self.line, PromptEvent::Update);
                 return EventResult::Consumed(None);

helix-term/tests/integration.rs

@@ -21,5 +21,6 @@ mod test {
     mod auto_pairs;
     mod commands;
     mod movement;
+    mod prompt;
     mod write;
 }

helix-term/tests/test/prompt.rs

@@ -0,0 +1,18 @@
+use super::*;
+
+use helix_term::application::Application;
+
+#[tokio::test]
+async fn test_history_completion() -> anyhow::Result<()> {
+    test_key_sequence(
+        &mut Application::new(Args::default(), Config::default())?,
+        Some(":asdf<ret>:theme d<C-n><tab>"),
+        Some(&|app| {
+            assert!(!app.editor.is_err());
+        }),
+        false,
+    )
+    .await?;
+
+    Ok(())
+}