Rework terminal highlight event flow

crates/terminal/src/terminal.rs

@@ -33,7 +33,6 @@ use mappings::mouse::{
 use procinfo::LocalProcessInfo;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use smol::channel::Sender;
 use util::truncate_and_trailoff;
 
 use std::{
@@ -90,12 +89,18 @@ pub enum Event {
     Wakeup,
     BlinkChanged,
     SelectionsChanged,
-    OpenUrl(String),
+    NewNavigationTarget(MaybeNavigationTarget),
-    ProbePathOpen {
+    Open(MaybeNavigationTarget),
-        maybe_path: String,
+}
-        can_open_tx: Sender<bool>,
+
-    },
+/// A string inside terminal, potentially useful as a URI that can be opened.
-    OpenPath(String),
+#[derive(Clone, Debug)]
+pub enum MaybeNavigationTarget {
+    /// HTTP, git, etc. string determined by the [`URL_REGEX`] regex.
+    Url(String),
+    /// File system path, absolute or relative, existing or not.
+    /// Might have line and column number(s) attached as `file.rs:1:23`
+    PathLike(String),
 }
 
 #[derive(Clone)]
@@ -502,6 +507,7 @@ impl TerminalBuilder {
             next_link_id: 0,
             selection_phase: SelectionPhase::Ended,
             cmd_pressed: false,
+            found_word: false,
         };
 
         Ok(TerminalBuilder {
@@ -654,6 +660,7 @@ pub struct Terminal {
     next_link_id: usize,
     selection_phase: SelectionPhase,
     cmd_pressed: bool,
+    found_word: bool,
 }
 
 impl Terminal {
@@ -834,7 +841,7 @@ impl Terminal {
                 .grid_clamp(term, alacritty_terminal::index::Boundary::Cursor);
 
                 let link = term.grid().index(point).hyperlink();
-                let found_url = if link.is_some() {
+                let found_word = if link.is_some() {
                     let mut min_index = point;
                     loop {
                         let new_min_index =
@@ -875,20 +882,21 @@ impl Terminal {
                     None
                 };
 
-                if let Some((maybe_url_or_path, is_url, url_match)) = found_url {
+                self.found_word = found_word.is_some();
+                if let Some((maybe_url_or_path, is_url, url_match)) = found_word {
                     if *open {
-                        let event = if is_url {
+                        let target = if is_url {
-                            Event::OpenUrl(maybe_url_or_path)
+                            MaybeNavigationTarget::Url(maybe_url_or_path)
                         } else {
-                            Event::OpenPath(maybe_url_or_path)
+                            MaybeNavigationTarget::PathLike(maybe_url_or_path)
                         };
-                        cx.emit(event);
+                        cx.emit(Event::Open(target));
                     } else {
                         self.update_selected_word(
                             prev_hovered_word,
-                            maybe_url_or_path,
                             url_match,
-                            !is_url,
+                            maybe_url_or_path,
+                            is_url,
                             cx,
                         );
                     }
@@ -900,9 +908,9 @@ impl Terminal {
     fn update_selected_word(
         &mut self,
         prev_word: Option<HoveredWord>,
-        word: String,
         word_match: RangeInclusive<Point>,
-        should_probe_word: bool,
+        word: String,
+        is_url: bool,
         cx: &mut ModelContext<Self>,
     ) {
         if let Some(prev_word) = prev_word {
@@ -912,43 +920,21 @@ impl Terminal {
                     word_match,
                     id: prev_word.id,
                 });
-            } else if should_probe_word {
+                return;
-                let (can_open_tx, can_open_rx) = smol::channel::bounded(1);
-                cx.emit(Event::ProbePathOpen {
-                    maybe_path: word.clone(),
-                    can_open_tx,
-                });
-
-                cx.spawn(|terminal, mut cx| async move {
-                    let can_open = can_open_rx.recv().await.unwrap_or(false);
-                    terminal.update(&mut cx, |terminal, cx| {
-                        if can_open {
-                            terminal.last_content.last_hovered_word = Some(HoveredWord {
-                                word,
-                                word_match,
-                                id: terminal.next_link_id(),
-                            });
-                        } else {
-                            terminal.last_content.last_hovered_word.take();
-                        }
-                        cx.notify();
-                    });
-                })
-                .detach();
-            } else {
-                self.last_content.last_hovered_word = Some(HoveredWord {
-                    word,
-                    word_match,
-                    id: self.next_link_id(),
-                });
             }
-        } else {
-            self.last_content.last_hovered_word = Some(HoveredWord {
-                word,
-                word_match,
-                id: self.next_link_id(),
-            });
         }
+
+        self.last_content.last_hovered_word = Some(HoveredWord {
+            word: word.clone(),
+            word_match,
+            id: self.next_link_id(),
+        });
+        let navigation_target = if is_url {
+            MaybeNavigationTarget::Url(word)
+        } else {
+            MaybeNavigationTarget::PathLike(word)
+        };
+        cx.emit(Event::NewNavigationTarget(navigation_target));
     }
 
     fn next_link_id(&mut self) -> usize {
@@ -1031,17 +1017,8 @@ impl Terminal {
     }
 
     pub fn try_modifiers_change(&mut self, modifiers: &Modifiers) -> bool {
-        let cmd = modifiers.cmd;
+        let changed = self.cmd_pressed != modifiers.cmd;
-        let changed = self.cmd_pressed != cmd;
+        self.cmd_pressed = modifiers.cmd;
-        if changed {
-            self.cmd_pressed = cmd;
-            if cmd {
-                self.refresh_hovered_word();
-            } else {
-                self.last_content.last_hovered_word.take();
-            }
-        }
-
         changed
     }
 
@@ -1336,7 +1313,7 @@ impl Terminal {
         }
     }
 
-    pub fn refresh_hovered_word(&mut self) {
+    fn refresh_hovered_word(&mut self) {
         self.word_from_position(self.last_mouse_position);
     }
 
@@ -1415,6 +1392,10 @@ impl Terminal {
             })
             .unwrap_or_else(|| "Terminal".to_string())
     }
+
+    pub fn can_navigate_to_selected_word(&self) -> bool {
+        self.cmd_pressed && self.found_word
+    }
 }
 
 impl Drop for Terminal {

crates/terminal_view/src/terminal_element.rs

@@ -163,6 +163,7 @@ pub struct TerminalElement {
     terminal: WeakModelHandle<Terminal>,
     focused: bool,
     cursor_visible: bool,
+    can_navigate_to_selected_word: bool,
 }
 
 impl TerminalElement {
@@ -170,11 +171,13 @@ impl TerminalElement {
         terminal: WeakModelHandle<Terminal>,
         focused: bool,
         cursor_visible: bool,
+        can_navigate_to_selected_word: bool,
     ) -> TerminalElement {
         TerminalElement {
             terminal,
             focused,
             cursor_visible,
+            can_navigate_to_selected_word,
         }
     }
 
@@ -580,13 +583,17 @@ impl Element<TerminalView> for TerminalElement {
         let background_color = terminal_theme.background;
         let terminal_handle = self.terminal.upgrade(cx).unwrap();
 
-        let last_hovered_hyperlink = terminal_handle.update(cx, |terminal, cx| {
+        let last_hovered_word = terminal_handle.update(cx, |terminal, cx| {
             terminal.set_size(dimensions);
             terminal.try_sync(cx);
-            terminal.last_content.last_hovered_word.clone()
+            if self.can_navigate_to_selected_word && terminal.can_navigate_to_selected_word() {
+                terminal.last_content.last_hovered_word.clone()
+            } else {
+                None
+            }
         });
 
-        let hyperlink_tooltip = last_hovered_hyperlink.map(|hovered_word| {
+        let hyperlink_tooltip = last_hovered_word.clone().map(|hovered_word| {
             let mut tooltip = Overlay::new(
                 Empty::new()
                     .contained()
@@ -619,7 +626,6 @@ impl Element<TerminalView> for TerminalElement {
             cursor_char,
             selection,
             cursor,
-            last_hovered_word,
             ..
         } = { &terminal_handle.read(cx).last_content };
 

crates/terminal_view/src/terminal_view.rs

@@ -33,7 +33,7 @@ use terminal::{
         index::Point,
         term::{search::RegexSearch, TermMode},
     },
-    Event, Terminal, TerminalBlink, WorkingDirectory,
+    Event, MaybeNavigationTarget, Terminal, TerminalBlink, WorkingDirectory,
 };
 use util::{paths::PathLikeWithPosition, ResultExt};
 use workspace::{
@@ -93,6 +93,7 @@ pub struct TerminalView {
     blinking_on: bool,
     blinking_paused: bool,
     blink_epoch: usize,
+    can_navigate_to_selected_word: bool,
     workspace_id: WorkspaceId,
 }
 
@@ -169,55 +170,61 @@ impl TerminalView {
                         .detach();
                 }
             }
-            Event::ProbePathOpen {
+            Event::NewNavigationTarget(maybe_navigation_target) => {
-                maybe_path,
+                this.can_navigate_to_selected_word = match maybe_navigation_target {
-                can_open_tx,
+                    MaybeNavigationTarget::Url(_) => true,
-            } => {
+                    MaybeNavigationTarget::PathLike(maybe_path) => {
-                let can_open = !possible_open_targets(&workspace, maybe_path, cx).is_empty();
+                        !possible_open_targets(&workspace, maybe_path, cx).is_empty()
-                can_open_tx.send_blocking(can_open).ok();
+                    }
-            }
-            Event::OpenUrl(url) => cx.platform().open_url(url),
-            Event::OpenPath(maybe_path) => {
-                let potential_abs_paths = possible_open_targets(&workspace, maybe_path, cx);
-                if let Some(path) = potential_abs_paths.into_iter().next() {
-                    let visible = path.path_like.is_dir();
-                    let task_workspace = workspace.clone();
-                    cx.spawn(|_, mut cx| async move {
-                        let opened_item = task_workspace
-                            .update(&mut cx, |workspace, cx| {
-                                workspace.open_abs_path(path.path_like, visible, cx)
-                            })
-                            .context("workspace update")?
-                            .await
-                            .context("workspace update")?;
-                        if let Some(row) = path.row {
-                            let col = path.column.unwrap_or(0);
-                            if let Some(active_editor) = opened_item.downcast::<Editor>() {
-                                active_editor
-                                    .downgrade()
-                                    .update(&mut cx, |editor, cx| {
-                                        let snapshot = editor.snapshot(cx).display_snapshot;
-                                        let point = snapshot.buffer_snapshot.clip_point(
-                                            language::Point::new(
-                                                row.saturating_sub(1),
-                                                col.saturating_sub(1),
-                                            ),
-                                            Bias::Left,
-                                        );
-                                        editor.change_selections(
-                                            Some(Autoscroll::center()),
-                                            cx,
-                                            |s| s.select_ranges([point..point]),
-                                        );
-                                    })
-                                    .log_err();
-                            }
-                        }
-                        anyhow::Ok(())
-                    })
-                    .detach_and_log_err(cx);
                 }
             }
+            Event::Open(maybe_navigation_target) => match maybe_navigation_target {
+                MaybeNavigationTarget::Url(url) => cx.platform().open_url(url),
+                MaybeNavigationTarget::PathLike(maybe_path) => {
+                    if !this.can_navigate_to_selected_word {
+                        return;
+                    }
+                    let potential_abs_paths = possible_open_targets(&workspace, maybe_path, cx);
+                    if let Some(path) = potential_abs_paths.into_iter().next() {
+                        let visible = path.path_like.is_dir();
+                        let task_workspace = workspace.clone();
+                        cx.spawn(|_, mut cx| async move {
+                            let opened_item = task_workspace
+                                .update(&mut cx, |workspace, cx| {
+                                    workspace.open_abs_path(path.path_like, visible, cx)
+                                })
+                                .context("workspace update")?
+                                .await
+                                .context("workspace update")?;
+                            if let Some(row) = path.row {
+                                let col = path.column.unwrap_or(0);
+                                if let Some(active_editor) = opened_item.downcast::<Editor>() {
+                                    active_editor
+                                        .downgrade()
+                                        .update(&mut cx, |editor, cx| {
+                                            let snapshot = editor.snapshot(cx).display_snapshot;
+                                            let point = snapshot.buffer_snapshot.clip_point(
+                                                language::Point::new(
+                                                    row.saturating_sub(1),
+                                                    col.saturating_sub(1),
+                                                ),
+                                                Bias::Left,
+                                            );
+                                            editor.change_selections(
+                                                Some(Autoscroll::center()),
+                                                cx,
+                                                |s| s.select_ranges([point..point]),
+                                            );
+                                        })
+                                        .log_err();
+                                }
+                            }
+                            anyhow::Ok(())
+                        })
+                        .detach_and_log_err(cx);
+                    }
+                }
+            },
             _ => cx.emit(event.clone()),
         })
         .detach();
@@ -231,6 +238,7 @@ impl TerminalView {
             blinking_on: false,
             blinking_paused: false,
             blink_epoch: 0,
+            can_navigate_to_selected_word: false,
             workspace_id,
         }
     }
@@ -466,6 +474,7 @@ impl View for TerminalView {
                     terminal_handle,
                     focused,
                     self.should_show_cursor(focused, cx),
+                    self.can_navigate_to_selected_word,
                 )
                 .contained(),
             )