Right-clicking to open the context menu leads to loss of code selection in the editor

[danny-su]

Check for existing issues

• [X] Completed

Describe the bug / provide steps to reproduce it

Right-clicking to open the context menu leads to loss of code selection in the editor (VIM mode enabled).

Environment

Zed: v0.118.1 (Zed) OS: macOS 14.2.1 Memory: 64 GiB Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

If you only need the most recent lines, you can run the zed: open log command palette action to see the last 1000.

No response

[ConradIrwin]

Updated to reflect that this happens regardless of vim mode. Thanks for the report!

[DivineDominion]

d796b543e0f2a8255e46b7862396f790f51317a2 added a selection clearing procedure that's still in effect:

    // Move the cursor to the clicked location so that dispatched actions make sense
    editor.change_selections(None, cx, |s| {
        s.clear_disjoint();
        s.set_pending_display_range(point..point, SelectMode::Character);
    });

Proposed solution

I'm not sure what to do with multiple selections when it comes to the context menu. The following may work:

• Did the user right-click a selected range?
  • Yes: Keep the selection as-is. Make cut/copy behave like the keyboard shortcuts would.
  • No: Discard the selection, set the insertion point at the clicked location. Expand the selection to the "word"-at-point. (See native macOS apps.) This clarifies what "Go to Definition" would act on.

Multi-selection and right-click to e.g. "Go to Definition" doesn't gel well. So these items should maybe be disabled in the long run when multiple ranges are selected.

Rebind to OsAction::Copy?

Searching the code I noticed that the ⌘C shortcut works fine and apparently uses

            MenuItem::os_action("Copy", editor::actions::Copy, OsAction::Copy),

according to crates/zed/src/zed/app_menus.rs, while the context menu is build via

    .action("Cut", Box::new(Cut))

in crates/editor/src/mouse_context_menu.rs.

Don't know if that's relevant, but wanted to leave this info here.

[ConradIrwin]

I think opening the menu should not change the selection.

The easiest fix would be to remove the check; I'm not sure we need to do more than that, as I think everything in there will work. (It's maybe a bit odd because some of it will act on the position of the mouse cursor, and some will act on the selection, but that seems better than the alternatives).

[DivineDominion]

@ConradIrwin I'll try removing the check 👍

Just for context, see the macOS default behavior from the video:

https://github.com/zed-industries/zed/assets/59080/592dacba-e2a1-4c93-ba5b-83cb0cca4e8e

Not changing the selection would differ from macOS platform expectations.

Not sure what other platforms do :)

[WeetHet]

d796b54 added a selection clearing procedure that's still in effect:

    // Move the cursor to the clicked location so that dispatched actions make sense
    editor.change_selections(None, cx, |s| {
        s.clear_disjoint();
        s.set_pending_display_range(point..point, SelectMode::Character);
    });

Proposed solution

I'm not sure what to do with multiple selections when it comes to the context menu. The following may work:

• Did the user right-click a selected range?

  • Yes: Keep the selection as-is. Make cut/copy behave like the keyboard shortcuts would.
  • No: Discard the selection, set the insertion point at the clicked location. Expand the selection to the "word"-at-point. (See native macOS apps.) This clarifies what "Go to Definition" would act on.

Multi-selection and right-click to e.g. "Go to Definition" doesn't gel well. So these items should maybe be disabled in the long run when multiple ranges are selected.

I agree with that this is what has to be done, would be nice to be able to do something with a mouse, for example when navigating the code