Fix next/prev shortcuts handling in the File Finder (#9785)

This PR fixes the unexpected File Finder behaviour described in
https://github.com/zed-industries/zed/pull/8782#issuecomment-2018551041

Any change of the modifier keys except for the release of the initial
modifier keys now prevents opening the selected file.

Release Notes:

- N/A

assets/keymaps/default-linux.json

@@ -561,7 +561,7 @@
   },
   {
     "context": "FileFinder",
-    "bindings": { "ctrl-shift-p": "menu::SelectPrev" }
+    "bindings": { "ctrl-shift-p": "file_finder::SelectPrev" }
   },
   {
     "context": "Terminal",

assets/keymaps/default-macos.json

@@ -600,7 +600,7 @@
   },
   {
     "context": "FileFinder",
-    "bindings": { "cmd-shift-p": "menu::SelectPrev" }
+    "bindings": { "cmd-shift-p": "file_finder::SelectPrev" }
   },
   {
     "context": "Terminal",

crates/file_finder/src/file_finder.rs

@@ -25,7 +25,7 @@ use ui::{prelude::*, HighlightedLabel, ListItem, ListItemSpacing};
 use util::{paths::PathLikeWithPosition, post_inc, ResultExt};
 use workspace::{ModalView, Workspace};
 
-actions!(file_finder, [Toggle]);
+actions!(file_finder, [Toggle, SelectPrev]);
 
 impl ModalView for FileFinder {}
 
@@ -47,9 +47,10 @@ impl FileFinder {
             };
 
             file_finder.update(cx, |file_finder, cx| {
-                file_finder
-                    .picker
-                    .update(cx, |picker, cx| picker.cycle_selection(cx))
+                file_finder.init_modifiers = Some(cx.modifiers());
+                file_finder.picker.update(cx, |picker, cx| {
+                    picker.cycle_selection(cx);
+                });
             });
         });
     }
@@ -105,7 +106,7 @@ impl FileFinder {
         event: &ModifiersChangedEvent,
         cx: &mut ViewContext<Self>,
     ) {
-        let Some(init_modifiers) = self.init_modifiers else {
+        let Some(init_modifiers) = self.init_modifiers.take() else {
             return;
         };
         if self.picker.read(cx).delegate.has_changed_selected_index {
@@ -115,6 +116,11 @@ impl FileFinder {
             }
         }
     }
+
+    fn handle_select_prev(&mut self, _: &SelectPrev, cx: &mut ViewContext<Self>) {
+        self.init_modifiers = Some(cx.modifiers());
+        cx.dispatch_action(Box::new(menu::SelectPrev));
+    }
 }
 
 impl EventEmitter<DismissEvent> for FileFinder {}
@@ -131,6 +137,7 @@ impl Render for FileFinder {
             .key_context("FileFinder")
             .w(rems(34.))
             .on_modifiers_changed(cx.listener(Self::handle_modifiers_changed))
+            .on_action(cx.listener(Self::handle_select_prev))
             .child(self.picker.clone())
     }
 }

crates/file_finder/src/file_finder_tests.rs

@@ -3,7 +3,7 @@ use std::{assert_eq, future::IntoFuture, path::Path, time::Duration};
 use super::*;
 use editor::Editor;
 use gpui::{Entity, TestAppContext, VisualTestContext};
-use menu::{Confirm, SelectNext};
+use menu::{Confirm, SelectNext, SelectPrev};
 use project::FS_WATCH_LATENCY;
 use serde_json::json;
 use workspace::{AppState, Workspace};
@@ -1535,6 +1535,120 @@ async fn test_opens_file_on_modifier_keys_release(cx: &mut gpui::TestAppContext)
     });
 }
 
+#[gpui::test]
+async fn test_switches_between_release_norelease_modes_on_forward_nav(
+    cx: &mut gpui::TestAppContext,
+) {
+    let app_state = init_test(cx);
+
+    app_state
+        .fs
+        .as_fake()
+        .insert_tree(
+            "/test",
+            json!({
+                "1.txt": "// One",
+                "2.txt": "// Two",
+            }),
+        )
+        .await;
+
+    let project = Project::test(app_state.fs.clone(), ["/test".as_ref()], cx).await;
+    let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project, cx));
+
+    open_queried_buffer("1", 1, "1.txt", &workspace, cx).await;
+    open_queried_buffer("2", 1, "2.txt", &workspace, cx).await;
+
+    // Open with a shortcut
+    cx.simulate_modifiers_change(Modifiers::command());
+    let picker = open_file_picker(&workspace, cx);
+    picker.update(cx, |finder, _| {
+        assert_eq!(finder.delegate.matches.len(), 2);
+        assert_match_selection(finder, 0, "2.txt");
+        assert_match_at_position(finder, 1, "1.txt");
+    });
+
+    // Switch to navigating with other shortcuts
+    // Don't open file on modifiers release
+    cx.simulate_modifiers_change(Modifiers::control());
+    cx.dispatch_action(SelectNext);
+    cx.simulate_modifiers_change(Modifiers::none());
+    picker.update(cx, |finder, _| {
+        assert_eq!(finder.delegate.matches.len(), 2);
+        assert_match_at_position(finder, 0, "2.txt");
+        assert_match_selection(finder, 1, "1.txt");
+    });
+
+    // Back to navigation with initial shortcut
+    // Open file on modifiers release
+    cx.simulate_modifiers_change(Modifiers::command());
+    cx.dispatch_action(Toggle);
+    cx.simulate_modifiers_change(Modifiers::none());
+    cx.read(|cx| {
+        let active_editor = workspace.read(cx).active_item_as::<Editor>(cx).unwrap();
+        assert_eq!(active_editor.read(cx).title(cx), "2.txt");
+    });
+}
+
+#[gpui::test]
+async fn test_switches_between_release_norelease_modes_on_backward_nav(
+    cx: &mut gpui::TestAppContext,
+) {
+    let app_state = init_test(cx);
+
+    app_state
+        .fs
+        .as_fake()
+        .insert_tree(
+            "/test",
+            json!({
+                "1.txt": "// One",
+                "2.txt": "// Two",
+                "3.txt": "// Three"
+            }),
+        )
+        .await;
+
+    let project = Project::test(app_state.fs.clone(), ["/test".as_ref()], cx).await;
+    let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project, cx));
+
+    open_queried_buffer("1", 1, "1.txt", &workspace, cx).await;
+    open_queried_buffer("2", 1, "2.txt", &workspace, cx).await;
+    open_queried_buffer("3", 1, "3.txt", &workspace, cx).await;
+
+    // Open with a shortcut
+    cx.simulate_modifiers_change(Modifiers::command());
+    let picker = open_file_picker(&workspace, cx);
+    picker.update(cx, |finder, _| {
+        assert_eq!(finder.delegate.matches.len(), 3);
+        assert_match_selection(finder, 0, "3.txt");
+        assert_match_at_position(finder, 1, "2.txt");
+        assert_match_at_position(finder, 2, "1.txt");
+    });
+
+    // Switch to navigating with other shortcuts
+    // Don't open file on modifiers release
+    cx.simulate_modifiers_change(Modifiers::control());
+    cx.dispatch_action(menu::SelectPrev);
+    cx.simulate_modifiers_change(Modifiers::none());
+    picker.update(cx, |finder, _| {
+        assert_eq!(finder.delegate.matches.len(), 3);
+        assert_match_at_position(finder, 0, "3.txt");
+        assert_match_at_position(finder, 1, "2.txt");
+        assert_match_selection(finder, 2, "1.txt");
+    });
+
+    // Back to navigation with initial shortcut
+    // Open file on modifiers release
+    cx.simulate_modifiers_change(Modifiers::command());
+    cx.dispatch_action(SelectPrev); // <-- File Finder's SelectPrev, not menu's
+    cx.simulate_modifiers_change(Modifiers::none());
+    cx.read(|cx| {
+        let active_editor = workspace.read(cx).active_item_as::<Editor>(cx).unwrap();
+        assert_eq!(active_editor.read(cx).title(cx), "3.txt");
+    });
+}
+
 #[gpui::test]
 async fn test_extending_modifiers_does_not_confirm_selection(cx: &mut gpui::TestAppContext) {
     let app_state = init_test(cx);

crates/gpui/src/platform/keystroke.rs

@@ -213,6 +213,14 @@ impl Modifiers {
         }
     }
 
+    /// helper method for Modifiers with just control
+    pub fn control() -> Modifiers {
+        Modifiers {
+            control: true,
+            ..Default::default()
+        }
+    }
+
     /// helper method for Modifiers with just shift
     pub fn shift() -> Modifiers {
         Modifiers {