zed icon indicating copy to clipboard operation
zed copied to clipboard

Add basic vi motion support for terminal

Open NukaCody opened this issue 1 year ago • 14 comments

Closes #7417

Release Notes:

  • Added basic support for Alacritty's vi mode to the built-in terminal (which is using Alacritty under the hood.) The vi mode can be activated with ctrl-shift-space and then supports some basic motions to navigate through the terminal's scrollback buffer.

Details

Leverages existing selection functionality from mouse_drag and the ViMotion API of alacritty to add basic vi motions in the terminal. Please note, this is only basic functionality (move, select, and yank to system clipboard) and not a fully functional vim environment (e.g. search, configurable keybindings, and paste). I figured this would be an interim solution to the long term, more fleshed out, solution proposed by @mrnugget.

Ctrl+Shift+Space to enter Vi mode while in the terminal (Same default binding in alacritty)

NukaCody avatar Oct 03 '24 22:10 NukaCody

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: flow. This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot[bot] avatar Oct 03 '24 22:10 cla-bot[bot]

We require contributors to sign our Contributor License Agreement, and we don't have @NukaCody on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

cla-bot[bot] avatar Oct 03 '24 23:10 cla-bot[bot]

@cla-bot check

NukaCody avatar Oct 03 '24 23:10 NukaCody

The cla-bot has been summoned, and re-checked this pull request!

cla-bot[bot] avatar Oct 03 '24 23:10 cla-bot[bot]

I can record a gif today showing the flow, it does work with surprisingly little code. It is missing some VIMotions though (SemanticRight/Semantic/Left) and (Top/Middle/Down). I mainly use it to grab a quick link or error message in the terminal but anything more I just pipe it to a file and use zed.

NukaCody avatar Oct 04 '24 14:10 NukaCody

2024-10-04 11 23 32

NukaCody avatar Oct 04 '24 15:10 NukaCody

There are limitations though that I've found while implementing, some of these are just foundational to how alacritty works while others is just to keep the commit small and iterate.

  • There is no coords (i.e. gg to go to top or vi" to get everything within the "", this is the same as alacritty)
  • High, Middle, and Low are not top,middle,low of the entire terminal, just what is viewable (I need to add this in a follow up commit)
  • This commit does not implement all of alacritty vi features (i.e. mainly searching with / and ?) Alacritty has more functionality here but I find myself just using /terminal or piping to a tmp file when I want to do anything more complicated cause having all of the zed tooling is better than basic vi support.

NukaCody avatar Oct 04 '24 15:10 NukaCody

Extended demo with scrolling motions

2024-10-04 17 06 32

NukaCody avatar Oct 04 '24 21:10 NukaCody

There is a minor bug I may need some help on, it seems that no matter what I try, whenever I use Page movements (PageDown, PageDownHalf, etc), selections are not updated until you input another vi_motion. Barely noticeable if you move fast enough but slightly annoying.

NukaCody avatar Oct 04 '24 21:10 NukaCody

h i, your demo can move the cursor freely on the print, I'm using vim-mode, but it seems like there's no way to make them compatible?

in ~.zshrc

export ZSH="$HOME/.oh-my-zsh"

plugins=(
  vi-mode
)

source $ZSH/oh-my-zsh.sh

bindkey -v
function zle-line-init zle-keymap-select {
    VIM_PROMPT="%{$fg_bold[yellow]%} [% NORMAL]%  %{$reset_color%}"
    # RPS1="${${KEYMAP/vicmd/$VIM_PROMPT}/(main|viins)/}$(git_custom_status) $EPS1"
    RPS1="${${KEYMAP/vicmd/$VIM_PROMPT}/(main|viins)/} $EPS1"
    zle reset-prompt
}
zle -N zle-line-init
zle -N zle-keymap-select

https://github.com/user-attachments/assets/a903c771-7842-4e76-84d0-bd49c511a53a

0x2CA avatar Oct 08 '24 12:10 0x2CA

Hey there! To activate vi_mode you have to press the "Control + Shift + Space" keys at the same time. This is the same key binding as alacritty (the underlying terminal used in zed)

Not sure if you did in the video (video shows a weird circle with an arrow through it so maybe you did?)

Right now the current implementation just passes everything through to the shell as is. Only after pressing "Control + Shift + Space" while the terminal is focused does vi_mode activate.

If you did press those keybinds and it doesn't work then I'm honestly not sure if this would be compatible with any zsh shell level plugin as most of the "vi" logic is actually implemented and deferred to the alacritty_terminal crate.

NukaCody avatar Oct 08 '24 16:10 NukaCody

h i, your demo can move the cursor freely on the print, I'm using vim-mode, but it seems like there's no way to make them compatible?

One is the Vim mode in the terminal emulator (Alacritty) and the other is the Vim mode in ZSH. They're distinct and don't know about each other. I'd be surprised if there's a non-clunky way to make them work together.


@NukaCody I haven't forgotten about this PR! Just need to find some time to review etc.

mrnugget avatar Oct 09 '24 07:10 mrnugget

I wanted to hop on your branch to make a few changes, but I can't push to your fork. So here's what I did:

  • Use our built-in actions to handle the toggling. I think this is cleaner and allows people to change the keybinding easily (ctrl-shift-space doesn't work for me, maybe another app is capturing it?, but ctrl-shift-v works)
  • Clean up some of the keystroke handling code.

Here is the commit I made locally:

commit 9ab99ec9670db2b783937794dc6ae36f17855a4d
Author: Thorsten Ball <[email protected]>
Date:   Wed Oct 9 12:28:00 2024 +0200

    Use action and simplify vi keystroke handling

diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json
index d33df02747..fca38a45a4 100644
--- a/assets/keymaps/default-linux.json
+++ b/assets/keymaps/default-linux.json
@@ -664,7 +664,8 @@
       "shift-up": "terminal::ScrollLineUp",
       "shift-down": "terminal::ScrollLineDown",
       "shift-home": "terminal::ScrollToTop",
-      "shift-end": "terminal::ScrollToBottom"
+      "shift-end": "terminal::ScrollToBottom",
+      "ctrl-shift-space": "terminal::ToggleViMode"
     }
   },
   {
diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json
index b405ee1852..053c25d9f1 100644
--- a/assets/keymaps/default-macos.json
+++ b/assets/keymaps/default-macos.json
@@ -673,7 +673,8 @@
       "cmd-home": "terminal::ScrollToTop",
       "cmd-end": "terminal::ScrollToBottom",
       "shift-home": "terminal::ScrollToTop",
-      "shift-end": "terminal::ScrollToBottom"
+      "shift-end": "terminal::ScrollToBottom",
+      "ctrl-shift-space": "terminal::ToggleViMode"
     }
   }
 ]
diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs
index 3b65fa16b1..d855b85045 100644
--- a/crates/terminal/src/terminal.rs
+++ b/crates/terminal/src/terminal.rs
@@ -79,6 +79,7 @@ actions!(
         ScrollPageDown,
         ScrollToTop,
         ScrollToBottom,
+        ToggleViMode,
     ]
 );
 
@@ -451,7 +452,7 @@ impl TerminalBuilder {
             hovered_word: false,
             url_regex,
             word_regex,
-            vi_mode: false,
+            vi_mode_enabled: false,
         };
 
         Ok(TerminalBuilder {
@@ -607,7 +608,7 @@ pub struct Terminal {
     url_regex: RegexSearch,
     word_regex: RegexSearch,
     task: Option<TaskState>,
-    vi_mode: bool,
+    vi_mode_enabled: bool,
 }
 
 pub struct TaskState {
@@ -774,7 +775,7 @@ impl Terminal {
                 term.scroll_display(*scroll);
                 self.refresh_hovered_word();
 
-                if self.vi_mode {
+                if self.vi_mode_enabled {
                     match *scroll {
                         AlacScroll::Delta(delta) => {
                             term.vi_mode_cursor = term.vi_mode_cursor.scroll(&term, delta);
@@ -842,6 +843,7 @@ impl Terminal {
                 self.refresh_hovered_word();
             }
             InternalEvent::ToggleViMode => {
+                self.vi_mode_enabled = !self.vi_mode_enabled;
                 term.toggle_vi_mode();
             }
             InternalEvent::ViMotion(motion) => {
@@ -1129,11 +1131,14 @@ impl Terminal {
     }
 
     pub fn toggle_vi_mode(&mut self) {
-        self.vi_mode = !self.vi_mode;
         self.events.push_back(InternalEvent::ToggleViMode);
     }
 
     pub fn vi_motion(&mut self, keystroke: &Keystroke) {
+        if !self.vi_mode_enabled {
+            return;
+        }
+
         let mut key = keystroke.key.clone();
         if keystroke.modifiers.shift {
             key = key.to_uppercase();
@@ -1157,38 +1162,30 @@ impl Terminal {
             _ => None,
         };
 
+        if let Some(motion) = motion {
+            let cursor = self.last_content.cursor.point;
+            let cursor_pos = Point {
+                x: cursor.column.0 as f32 * self.last_content.size.cell_width,
+                y: cursor.line.0 as f32 * self.last_content.size.line_height,
+            };
+            self.events
+                .push_back(InternalEvent::UpdateSelection(cursor_pos));
+            self.events.push_back(InternalEvent::ViMotion(motion));
+            return;
+        }
+
         let scroll_motion = match key.as_str() {
             "g" => Some(AlacScroll::Top),
             "G" => Some(AlacScroll::Bottom),
-            "b" => {
-                if keystroke.modifiers.control {
-                    Some(AlacScroll::PageUp)
-                } else {
-                    None
-                }
+            "b" if keystroke.modifiers.control => Some(AlacScroll::PageUp),
+            "f" if keystroke.modifiers.control => Some(AlacScroll::PageDown),
+            "d" if keystroke.modifiers.control => {
+                let amount = self.last_content.size.line_height().to_f64() as i32 / 2;
+                Some(AlacScroll::Delta(-amount))
             }
-            "f" => {
-                if keystroke.modifiers.control {
-                    Some(AlacScroll::PageDown)
-                } else {
-                    None
-                }
-            }
-            "d" => {
-                if keystroke.modifiers.control {
-                    let amount = self.last_content.size.line_height().to_f64() as i32 / 2;
-                    Some(AlacScroll::Delta(-amount))
-                } else {
-                    None
-                }
-            }
-            "u" => {
-                if keystroke.modifiers.control {
-                    let amount = self.last_content.size.line_height().to_f64() as i32 / 2;
-                    Some(AlacScroll::Delta(amount))
-                } else {
-                    None
-                }
+            "u" if keystroke.modifiers.control => {
+                let amount = self.last_content.size.line_height().to_f64() as i32 / 2;
+                Some(AlacScroll::Delta(amount))
             }
             _ => None,
         };
@@ -1198,57 +1195,39 @@ impl Terminal {
             return;
         }
 
-        if keystroke.key.as_str() == "v" {
-            let point = self.last_content.cursor.point;
-            let selection_type = SelectionType::Simple;
-            let side = AlacDirection::Right;
-            let selection = Selection::new(selection_type, point, side);
-            self.events
-                .push_back(InternalEvent::SetSelection(Some((selection, point))));
-            return;
-        }
+        match key.as_str() {
+            "v" => {
+                let point = self.last_content.cursor.point;
+                let selection_type = SelectionType::Simple;
+                let side = AlacDirection::Right;
+                let selection = Selection::new(selection_type, point, side);
+                self.events
+                    .push_back(InternalEvent::SetSelection(Some((selection, point))));
+                return;
+            }
 
-        if keystroke.key.as_str() == "escape" {
-            self.events.push_back(InternalEvent::SetSelection(None));
-            return;
-        }
+            "escape" => {
+                self.events.push_back(InternalEvent::SetSelection(None));
+                return;
+            }
 
-        if keystroke.key.as_str() == "y" {
-            self.events.push_back(InternalEvent::Copy);
-            self.events.push_back(InternalEvent::SetSelection(None));
-            return;
-        }
+            "y" => {
+                self.events.push_back(InternalEvent::Copy);
+                self.events.push_back(InternalEvent::SetSelection(None));
+                return;
+            }
 
-        if keystroke.key.as_str() == "i" {
-            if self.vi_mode {
+            "i" => {
                 self.scroll_to_bottom();
                 self.toggle_vi_mode();
+                return;
             }
-            return;
-        }
-
-        if let Some(motion) = motion {
-            let cursor = self.last_content.cursor.point;
-            let cursor_pos = Point {
-                x: cursor.column.0 as f32 * self.last_content.size.cell_width,
-                y: cursor.line.0 as f32 * self.last_content.size.line_height,
-            };
-            self.events
-                .push_back(InternalEvent::UpdateSelection(cursor_pos));
-            self.events.push_back(InternalEvent::ViMotion(motion));
+            _ => {}
         }
     }
 
     pub fn try_keystroke(&mut self, keystroke: &Keystroke, alt_is_meta: bool) -> bool {
-        if keystroke.modifiers.control
-            && keystroke.modifiers.shift
-            && keystroke.key.as_str() == "space"
-        {
-            self.toggle_vi_mode();
-            return true;
-        }
-
-        if self.vi_mode {
+        if self.vi_mode_enabled {
             self.vi_motion(keystroke);
             return true;
         }
diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs
index ce65be30c6..28a3d61d65 100644
--- a/crates/terminal_view/src/terminal_view.rs
+++ b/crates/terminal_view/src/terminal_view.rs
@@ -22,7 +22,7 @@ use terminal::{
     terminal_settings::{CursorShape, TerminalBlink, TerminalSettings, WorkingDirectory},
     Clear, Copy, Event, MaybeNavigationTarget, Paste, ScrollLineDown, ScrollLineUp, ScrollPageDown,
     ScrollPageUp, ScrollToBottom, ScrollToTop, ShowCharacterPalette, TaskStatus, Terminal,
-    TerminalSize,
+    TerminalSize, ToggleViMode,
 };
 use terminal_element::{is_blank, TerminalElement};
 use terminal_panel::TerminalPanel;
@@ -431,6 +431,11 @@ impl TerminalView {
         cx.notify();
     }
 
+    fn toggle_vi_mode(&mut self, _: &ToggleViMode, cx: &mut ViewContext<Self>) {
+        self.terminal.update(cx, |term, _| term.toggle_vi_mode());
+        cx.notify();
+    }
+
     pub fn should_show_cursor(&self, focused: bool, cx: &mut gpui::ViewContext<Self>) -> bool {
         //Don't blink the cursor when not focused, blinking is disabled, or paused
         if !focused
@@ -968,6 +973,7 @@ impl Render for TerminalView {
             .on_action(cx.listener(TerminalView::scroll_page_down))
             .on_action(cx.listener(TerminalView::scroll_to_top))
             .on_action(cx.listener(TerminalView::scroll_to_bottom))
+            .on_action(cx.listener(TerminalView::toggle_vi_mode))
             .on_action(cx.listener(TerminalView::show_character_palette))
             .on_action(cx.listener(TerminalView::select_all))
             .on_key_down(cx.listener(Self::key_down))

You can either manually pull over those changes, or save as foo.patch and git apply foo.patch

mrnugget avatar Oct 09 '24 10:10 mrnugget

Love these changes, will apply them tonight

NukaCody avatar Oct 09 '24 16:10 NukaCody

Alright, since it's small and self-contained, let's go ahead with this!

mrnugget avatar Oct 10 '24 05:10 mrnugget

Hey, thanks @NukaCody and @mrnugget ! Seems that all basic vi motions are here and a ton of usefulness in a small amount of code. Great job!

baldwindavid avatar Oct 16 '24 20:10 baldwindavid

I was so glad to see this in the latest release notes, I've missed being able to easily navigate the terminal's scrollback buffer! Thank you so much for working on this @NukaCody and @mrnugget ! 💙

dinocosta avatar Oct 24 '24 10:10 dinocosta