Repository: alacritty/alacritty (language: rust)

Issue:
Highlighted hints use old data for their actions
Currently the following logic is used to open hints with the mouse/vi mode:

```rust
                let hint = ctx.display().vi_highlighted_hint.take();
                if let Some(hint) = &hint {
                    ctx.trigger_hint(hint);
                }
                ctx.display().vi_highlighted_hint = hint;
```

```rust
        let hint = self.ctx.display().highlighted_hint.take();
        if let Some(hint) = hint.as_ref().filter(|_| button == MouseButton::Left) {
            self.ctx.trigger_hint(hint);
        }
        self.ctx.display().highlighted_hint = hint;
```

Both of these take the current highlighted hint and pass it directly to `trigger_hint`. However `trigger_hint` uses the following logic to convert the hint to its text equivalent:

```rust
        let hint_bounds = hint.bounds();
        let text = match hint.hyperlink() {
            Some(hyperlink) => hyperlink.uri().to_owned(),
            None => self.terminal.bounds_to_string(*hint_bounds.start(), *hint_bounds.end()),
        };
```

This is problematic since the highlighted hint has not been checked for validity since the last frame, which means the underlying terminal content could have changed since the user requested its opening.

To avoid accidentally opening arbitrary text, Alacritty should either invalidate the highlighted hints before passing them to the `trigger_hint` function or recreate the hints.

If the hints are recreated, we might also be able to remove the ugly `take()` -> `assign` stuff.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1c28ddd10f9..bbe0d2f4f0f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,7 @@ Notable changes to the `alacritty_terminal` crate are documented in its
 ### Fixed
 
 - Mouse/Vi cursor hint highlighting broken on the terminal cursor line
+- Hint launcher opening arbitrary text, when terminal content changed while opening
 
 ## 0.14.0
 
diff --git a/alacritty/src/config/bindings.rs b/alacritty/src/config/bindings.rs
index dfe31853bc0..931b0583ba2 100644
--- a/alacritty/src/config/bindings.rs
+++ b/alacritty/src/config/bindings.rs
@@ -5,6 +5,7 @@ use std::fmt::{self, Debug, Display};
 use bitflags::bitflags;
 use serde::de::{self, Error as SerdeError, MapAccess, Unexpected, Visitor};
 use serde::{Deserialize, Deserializer};
+use std::rc::Rc;
 use toml::Value as SerdeValue;
 use winit::event::MouseButton;
 use winit::keyboard::{
@@ -96,7 +97,7 @@ pub enum Action {
 
     /// Regex keyboard hints.
     #[config(skip)]
-    Hint(Hint),
+    Hint(Rc<Hint>),
 
     /// Move vi mode cursor.
     #[config(skip)]
@@ -790,7 +791,7 @@ impl<'a> Deserialize<'a> for ModeWrapper {
     {
         struct ModeVisitor;
 
-        impl<'a> Visitor<'a> for ModeVisitor {
+        impl Visitor<'_> for ModeVisitor {
             type Value = ModeWrapper;
 
             fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -844,7 +845,7 @@ impl<'a> Deserialize<'a> for MouseButtonWrapper {
     {
         struct MouseButtonVisitor;
 
-        impl<'a> Visitor<'a> for MouseButtonVisitor {
+        impl Visitor<'_> for MouseButtonVisitor {
             type Value = MouseButtonWrapper;
 
             fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -956,7 +957,7 @@ impl<'a> Deserialize<'a> for RawBinding {
             {
                 struct FieldVisitor;
 
-                impl<'a> Visitor<'a> for FieldVisitor {
+                impl Visitor<'_> for FieldVisitor {
                     type Value = Field;
 
                     fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -1204,7 +1205,7 @@ impl<'a> de::Deserialize<'a> for ModsWrapper {
     {
         struct ModsVisitor;
 
-        impl<'a> Visitor<'a> for ModsVisitor {
+        impl Visitor<'_> for ModsVisitor {
             type Value = ModsWrapper;
 
             fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
diff --git a/alacritty/src/config/font.rs b/alacritty/src/config/font.rs
index 061c0f42a36..760c26d5832 100644
--- a/alacritty/src/config/font.rs
+++ b/alacritty/src/config/font.rs
@@ -144,7 +144,7 @@ impl<'de> Deserialize<'de> for Size {
         D: Deserializer<'de>,
     {
         struct NumVisitor;
-        impl<'v> Visitor<'v> for NumVisitor {
+        impl Visitor<'_> for NumVisitor {
             type Value = Size;
 
             fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
diff --git a/alacritty/src/config/ui_config.rs b/alacritty/src/config/ui_config.rs
index 69716dee3cf..b44bda0d757 100644
--- a/alacritty/src/config/ui_config.rs
+++ b/alacritty/src/config/ui_config.rs
@@ -253,7 +253,7 @@ pub struct Hints {
     alphabet: HintsAlphabet,
 
     /// All configured terminal hints.
-    pub enabled: Vec<Hint>,
+    pub enabled: Vec<Rc<Hint>>,
 }
 
 impl Default for Hints {
@@ -274,7 +274,7 @@ impl Default for Hints {
         });
 
         Self {
-            enabled: vec![Hint {
+            enabled: vec![Rc::new(Hint {
                 content,
                 action,
                 persist: false,
@@ -288,7 +288,7 @@ impl Default for Hints {
                     mods: ModsWrapper(ModifiersState::SHIFT | ModifiersState::CONTROL),
                     mode: Default::default(),
                 }),
-            }],
+            })],
             alphabet: Default::default(),
         }
     }
@@ -619,7 +619,7 @@ impl SerdeReplace for Program {
 }
 
 pub(crate) struct StringVisitor;
-impl<'de> serde::de::Visitor<'de> for StringVisitor {
+impl serde::de::Visitor<'_> for StringVisitor {
     type Value = String;
 
     fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
diff --git a/alacritty/src/display/color.rs b/alacritty/src/display/color.rs
index 669bf502f89..2e854a3fa12 100644
--- a/alacritty/src/display/color.rs
+++ b/alacritty/src/display/color.rs
@@ -18,7 +18,7 @@ pub const DIM_FACTOR: f32 = 0.66;
 #[derive(Copy, Clone)]
 pub struct List([Rgb; COUNT]);
 
-impl<'a> From<&'a Colors> for List {
+impl From<&'_ Colors> for List {
     fn from(colors: &Colors) -> List {
         // Type inference fails without this annotation.
         let mut list = List([Rgb::default(); COUNT]);
@@ -235,7 +235,7 @@ impl<'de> Deserialize<'de> for Rgb {
             b: u8,
         }
 
-        impl<'a> Visitor<'a> for RgbVisitor {
+        impl Visitor<'_> for RgbVisitor {
             type Value = Rgb;
 
             fn expecting(&self, f: &mut Formatter<'_>) -> fmt::Result {
@@ -331,7 +331,7 @@ impl<'de> Deserialize<'de> for CellRgb {
         const EXPECTING: &str = "CellForeground, CellBackground, or hex color like #ff00ff";
 
         struct CellRgbVisitor;
-        impl<'a> Visitor<'a> for CellRgbVisitor {
+        impl Visitor<'_> for CellRgbVisitor {
             type Value = CellRgb;
 
             fn expecting(&self, f: &mut Formatter<'_>) -> fmt::Result {
diff --git a/alacritty/src/display/content.rs b/alacritty/src/display/content.rs
index 2fbbdec42ae..edd2709f063 100644
--- a/alacritty/src/display/content.rs
+++ b/alacritty/src/display/content.rs
@@ -150,7 +150,7 @@ impl<'a> RenderableContent<'a> {
     }
 }
 
-impl<'a> Iterator for RenderableContent<'a> {
+impl Iterator for RenderableContent<'_> {
     type Item = RenderableCell;
 
     /// Gets the next renderable cell.
@@ -453,7 +453,7 @@ struct Hint<'a> {
     labels: &'a Vec<Vec<char>>,
 }
 
-impl<'a> Hint<'a> {
+impl Hint<'_> {
     /// Advance the hint iterator.
     ///
     /// If the point is within a hint, the keyboard shortcut character that should be displayed at
@@ -543,7 +543,7 @@ impl<'a> HintMatches<'a> {
     }
 }
 
-impl<'a> Deref for HintMatches<'a> {
+impl Deref for HintMatches<'_> {
     type Target = [Match];
 
     fn deref(&self) -> &Self::Target {
diff --git a/alacritty/src/display/damage.rs b/alacritty/src/display/damage.rs
index fc6aef39f32..b07363753f7 100644
--- a/alacritty/src/display/damage.rs
+++ b/alacritty/src/display/damage.rs
@@ -251,7 +251,7 @@ impl<'a> RenderDamageIterator<'a> {
     }
 }
 
-impl<'a> Iterator for RenderDamageIterator<'a> {
+impl Iterator for RenderDamageIterator<'_> {
     type Item = Rect;
 
     fn next(&mut self) -> Option<Rect> {
diff --git a/alacritty/src/display/hint.rs b/alacritty/src/display/hint.rs
index a01a1d03cf9..3f10b4e5639 100644
--- a/alacritty/src/display/hint.rs
+++ b/alacritty/src/display/hint.rs
@@ -1,6 +1,8 @@
+use std::borrow::Cow;
 use std::cmp::Reverse;
 use std::collections::HashSet;
 use std::iter;
+use std::rc::Rc;
 
 use ahash::RandomState;
 use winit::keyboard::ModifiersState;
@@ -23,7 +25,7 @@ const HINT_SPLIT_PERCENTAGE: f32 = 0.5;
 /// Keyboard regex hint state.
 pub struct HintState {
     /// Hint currently in use.
-    hint: Option<Hint>,
+    hint: Option<Rc<Hint>>,
 
     /// Alphabet for hint labels.
     alphabet: String,
@@ -56,7 +58,7 @@ impl HintState {
     }
 
     /// Start the hint selection process.
-    pub fn start(&mut self, hint: Hint) {
+    pub fn start(&mut self, hint: Rc<Hint>) {
         self.hint = Some(hint);
     }
 
@@ -150,7 +152,7 @@ impl HintState {
         // Check if the selected label is fully matched.
         if label.len() == 1 {
             let bounds = self.matches[index].clone();
-            let action = hint.action.clone();
+            let hint = hint.clone();
 
             // Exit hint mode unless it requires explicit dismissal.
             if hint.persist {
@@ -161,7 +163,7 @@ impl HintState {
 
             // Hyperlinks take precedence over regex matches.
             let hyperlink = term.grid()[*bounds.start()].hyperlink();
-            Some(HintMatch { action, bounds, hyperlink })
+            Some(HintMatch { bounds, hyperlink, hint })
         } else {
             // Store character to preserve the selection.
             self.keys.push(c);
@@ -192,13 +194,14 @@ impl HintState {
 /// Hint match which was selected by the user.
 #[derive(PartialEq, Eq, Debug, Clone)]
 pub struct HintMatch {
-    /// Action for handling the text.
-    action: HintAction,
-
     /// Terminal range matching the hint.
     bounds: Match,
 
+    /// OSC 8 hyperlink.
     hyperlink: Option<Hyperlink>,
+
+    /// Hint which triggered this match.
+    hint: Rc<Hint>,
 }
 
 impl HintMatch {
@@ -210,7 +213,7 @@ impl HintMatch {
 
     #[inline]
     pub fn action(&self) -> &HintAction {
-        &self.action
+        &self.hint.action
     }
 
     #[inline]
@@ -221,6 +224,29 @@ impl HintMatch {
     pub fn hyperlink(&self) -> Option<&Hyperlink> {
         self.hyperlink.as_ref()
     }
+
+    /// Get the text content of the hint match.
+    ///
+    /// This will always revalidate the hint text, to account for terminal content
+    /// changes since the [`HintMatch`] was constructed. The text of the hint might
+    /// be different from its original value, but it will **always** be a valid
+    /// match for this hint.
+    pub fn text<T>(&self, term: &Term<T>) -> Option<Cow<'_, str>> {
+        // Revalidate hyperlink match.
+        if let Some(hyperlink) = &self.hyperlink {
+            let (validated, bounds) = hyperlink_at(term, *self.bounds.start())?;
+            return (&validated == hyperlink && bounds == self.bounds)
+                .then(|| hyperlink.uri().into());
+        }
+
+        // Revalidate regex match.
+        let regex = self.hint.content.regex.as_ref()?;
+        let bounds = regex.with_compiled(|regex| {
+            regex_match_at(term, *self.bounds.start(), regex, self.hint.post_processing)
+        })??;
+        (bounds == self.bounds)
+            .then(|| term.bounds_to_string(*bounds.start(), *bounds.end()).into())
+    }
 }
 
 /// Generator for creating new hint labels.
@@ -382,18 +408,14 @@ pub fn highlighted_at<T>(
         if let Some((hyperlink, bounds)) =
             hint.content.hyperlinks.then(|| hyperlink_at(term, point)).flatten()
         {
-            return Some(HintMatch {
-                bounds,
-                action: hint.action.clone(),
-                hyperlink: Some(hyperlink),
-            });
+            return Some(HintMatch { bounds, hyperlink: Some(hyperlink), hint: hint.clone() });
         }
 
         let bounds = hint.content.regex.as_ref().and_then(|regex| {
             regex.with_compiled(|regex| regex_match_at(term, point, regex, hint.post_processing))
         });
         if let Some(bounds) = bounds.flatten() {
-            return Some(HintMatch { bounds, action: hint.action.clone(), hyperlink: None });
+            return Some(HintMatch { bounds, hint: hint.clone(), hyperlink: None });
         }
 
         None
@@ -554,7 +576,7 @@ impl<'a, T> HintPostProcessor<'a, T> {
     }
 }
 
-impl<'a, T> Iterator for HintPostProcessor<'a, T> {
+impl<T> Iterator for HintPostProcessor<'_, T> {
     type Item = Match;
 
     fn next(&mut self) -> Option<Self::Item> {
diff --git a/alacritty/src/display/meter.rs b/alacritty/src/display/meter.rs
index e263100fdfb..20cbbd1b172 100644
--- a/alacritty/src/display/meter.rs
+++ b/alacritty/src/display/meter.rs
@@ -57,7 +57,7 @@ impl<'a> Sampler<'a> {
     }
 }
 
-impl<'a> Drop for Sampler<'a> {
+impl Drop for Sampler<'_> {
     fn drop(&mut self) {
         self.meter.add_sample(self.alive_duration());
     }
diff --git a/alacritty/src/event.rs b/alacritty/src/event.rs
index f01590603b2..e82d797475f 100644
--- a/alacritty/src/event.rs
+++ b/alacritty/src/event.rs
@@ -1141,16 +1141,16 @@ impl<'a, N: Notify + 'a, T: EventListener> input::ActionContext<T> for ActionCon
         }
 
         let hint_bounds = hint.bounds();
-        let text = match hint.hyperlink() {
-            Some(hyperlink) => hyperlink.uri().to_owned(),
-            None => self.terminal.bounds_to_string(*hint_bounds.start(), *hint_bounds.end()),
+        let text = match hint.text(self.terminal) {
+            Some(text) => text,
+            None => return,
         };
 
         match &hint.action() {
             // Launch an external program.
             HintAction::Command(command) => {
                 let mut args = command.args().to_vec();
-                args.push(text);
+                args.push(text.into());
                 self.spawn_daemon(command.program(), &args);
             },
             // Copy the text to the clipboard.
diff --git a/alacritty/src/input/mod.rs b/alacritty/src/input/mod.rs
index bbd8673f7bb..1c9d6008e07 100644
--- a/alacritty/src/input/mod.rs
+++ b/alacritty/src/input/mod.rs
@@ -1136,7 +1136,7 @@ mod tests {
         inline_search_state: &'a mut InlineSearchState,
     }
 
-    impl<'a, T: EventListener> super::ActionContext<T> for ActionContext<'a, T> {
+    impl<T: EventListener> super::ActionContext<T> for ActionContext<'_, T> {
         fn search_next(
             &mut self,
             _origin: Point,
diff --git a/alacritty/src/migrate/mod.rs b/alacritty/src/migrate/mod.rs
index 2f806d361a1..42b51d27792 100644
--- a/alacritty/src/migrate/mod.rs
+++ b/alacritty/src/migrate/mod.rs
@@ -244,7 +244,7 @@ enum Migration<'a> {
     Yaml((&'a Path, String)),
 }
 
-impl<'a> Migration<'a> {
+impl Migration<'_> {
     /// Get the success message for this migration.
     fn success_message(&self, import: bool) -> String {
         match self {
diff --git a/alacritty/src/renderer/text/gles2.rs b/alacritty/src/renderer/text/gles2.rs
index 427a60e397f..8756ea80d10 100644
--- a/alacritty/src/renderer/text/gles2.rs
+++ b/alacritty/src/renderer/text/gles2.rs
@@ -346,7 +346,7 @@ pub struct RenderApi<'a> {
     dual_source_blending: bool,
 }
 
-impl<'a> Drop for RenderApi<'a> {
+impl Drop for RenderApi<'_> {
     fn drop(&mut self) {
         if !self.batch.is_empty() {
             self.render_batch();
@@ -354,7 +354,7 @@ impl<'a> Drop for RenderApi<'a> {
     }
 }
 
-impl<'a> LoadGlyph for RenderApi<'a> {
+impl LoadGlyph for RenderApi<'_> {
     fn load_glyph(&mut self, rasterized: &RasterizedGlyph) -> Glyph {
         Atlas::load_glyph(self.active_tex, self.atlas, self.current_atlas, rasterized)
     }
@@ -364,7 +364,7 @@ impl<'a> LoadGlyph for RenderApi<'a> {
     }
 }
 
-impl<'a> TextRenderApi<Batch> for RenderApi<'a> {
+impl TextRenderApi<Batch> for RenderApi<'_> {
     fn batch(&mut self) -> &mut Batch {
         self.batch
     }
diff --git a/alacritty/src/renderer/text/glsl3.rs b/alacritty/src/renderer/text/glsl3.rs
index 7c32bf9f9e9..fee95ce32d8 100644
--- a/alacritty/src/renderer/text/glsl3.rs
+++ b/alacritty/src/renderer/text/glsl3.rs
@@ -215,7 +215,7 @@ pub struct RenderApi<'a> {
     program: &'a mut TextShaderProgram,
 }
 
-impl<'a> TextRenderApi<Batch> for RenderApi<'a> {
+impl TextRenderApi<Batch> for RenderApi<'_> {
     fn batch(&mut self) -> &mut Batch {
         self.batch
     }
@@ -261,7 +261,7 @@ impl<'a> TextRenderApi<Batch> for RenderApi<'a> {
     }
 }
 
-impl<'a> LoadGlyph for RenderApi<'a> {
+impl LoadGlyph for RenderApi<'_> {
     fn load_glyph(&mut self, rasterized: &RasterizedGlyph) -> Glyph {
         Atlas::load_glyph(self.active_tex, self.atlas, self.current_atlas, rasterized)
     }
@@ -271,7 +271,7 @@ impl<'a> LoadGlyph for RenderApi<'a> {
     }
 }
 
-impl<'a> Drop for RenderApi<'a> {
+impl Drop for RenderApi<'_> {
     fn drop(&mut self) {
         if !self.batch.is_empty() {
             self.render_batch();
diff --git a/alacritty/src/renderer/text/mod.rs b/alacritty/src/renderer/text/mod.rs
index 886b7f8bd49..acd1b521673 100644
--- a/alacritty/src/renderer/text/mod.rs
+++ b/alacritty/src/renderer/text/mod.rs
@@ -186,7 +186,7 @@ pub struct LoaderApi<'a> {
     current_atlas: &'a mut usize,
 }
 
-impl<'a> LoadGlyph for LoaderApi<'a> {
+impl LoadGlyph for LoaderApi<'_> {
     fn load_glyph(&mut self, rasterized: &RasterizedGlyph) -> Glyph {
         Atlas::load_glyph(self.active_tex, self.atlas, self.current_atlas, rasterized)
     }
diff --git a/alacritty/src/string.rs b/alacritty/src/string.rs
index a7af439465d..b8c47d3bd4d 100644
--- a/alacritty/src/string.rs
+++ b/alacritty/src/string.rs
@@ -106,7 +106,7 @@ impl<'a> StrShortener<'a> {
     }
 }
 
-impl<'a> Iterator for StrShortener<'a> {
+impl Iterator for StrShortener<'_> {
     type Item = char;
 
     fn next(&mut self) -> Option<Self::Item> {
diff --git a/alacritty_terminal/src/grid/mod.rs b/alacritty_terminal/src/grid/mod.rs
index 278bea3bc17..fbd2c79eb41 100644
--- a/alacritty_terminal/src/grid/mod.rs
+++ b/alacritty_terminal/src/grid/mod.rs
@@ -615,7 +615,7 @@ pub trait BidirectionalIterator: Iterator {
     fn prev(&mut self) -> Option<Self::Item>;
 }
 
-impl<'a, T> BidirectionalIterator for GridIterator<'a, T> {
+impl<T> BidirectionalIterator for GridIterator<'_, T> {
     fn prev(&mut self) -> Option<Self::Item> {
         let topmost_line = self.grid.topmost_line();
         let last_column = self.grid.last_column();
diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs
index 8a0410dc343..1f1e52c1cfb 100644
--- a/alacritty_terminal/src/term/mod.rs
+++ b/alacritty_terminal/src/term/mod.rs
@@ -199,7 +199,7 @@ impl<'a> TermDamageIterator<'a> {
     }
 }
 
-impl<'a> Iterator for TermDamageIterator<'a> {
+impl Iterator for TermDamageIterator<'_> {
     type Item = LineDamageBounds;
 
     fn next(&mut self) -> Option<Self::Item> {
diff --git a/alacritty_terminal/src/term/search.rs b/alacritty_terminal/src/term/search.rs
index 33f6ee056b5..20a427b707b 100644
--- a/alacritty_terminal/src/term/search.rs
+++ b/alacritty_terminal/src/term/search.rs
@@ -656,7 +656,7 @@ impl<'a, T> RegexIter<'a, T> {
     }
 }
 
-impl<'a, T> Iterator for RegexIter<'a, T> {
+impl<T> Iterator for RegexIter<'_, T> {
     type Item = Match;
 
     fn next(&mut self) -> Option<Self::Item> {


Test patch: