From 40bbdce6de8775b7bbc3f9a5731337d1dd05d195 Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Mon, 11 Jul 2022 22:54:28 +0300 Subject: [PATCH] Fix hyperlinks not being keyboard actionable This fixes a typo in 694a52b which was filtering all hyperlinks, except only duplicated ones when opening them with keyboard. Co-authored-by: Christian Duerr --- alacritty/src/display/hint.rs | 44 +++++++++++++++++++++++++-- alacritty_terminal/src/event.rs | 7 +++-- alacritty_terminal/src/term/mod.rs | 48 ++++++++++++++++-------------- alacritty_terminal/src/vi_mode.rs | 7 +++-- 4 files changed, 74 insertions(+), 32 deletions(-) diff --git a/alacritty/src/display/hint.rs b/alacritty/src/display/hint.rs index a59f2bc7..887840e9 100644 --- a/alacritty/src/display/hint.rs +++ b/alacritty/src/display/hint.rs @@ -79,7 +79,7 @@ impl HintState { // Add escape sequence hyperlinks. if hint.content.hyperlinks { - self.matches.extend(visible_unique_hyperlink_iter(term)); + self.matches.extend(visible_unique_hyperlinks_iter(term)); } // Add visible regex matches. @@ -298,7 +298,7 @@ pub fn visible_regex_match_iter<'a, T>( } /// Iterate over all visible hyperlinks, yanking only unique ones. -pub fn visible_unique_hyperlink_iter(term: &Term) -> impl Iterator + '_ { +pub fn visible_unique_hyperlinks_iter(term: &Term) -> impl Iterator + '_ { let mut display_iter = term.grid().display_iter().peekable(); // Avoid creating hints for the same hyperlinks, but from a different places. @@ -308,7 +308,7 @@ pub fn visible_unique_hyperlink_iter(term: &Term) -> impl Iterator Iterator for HintPostProcessor<'a, T> { #[cfg(test)] mod tests { + use alacritty_terminal::ansi::Handler; use alacritty_terminal::index::{Column, Line}; use alacritty_terminal::term::test::mock_term; @@ -642,4 +643,41 @@ mod tests { assert_eq!(count, 0); } + + #[test] + fn collect_unique_hyperlinks() { + let mut term = mock_term("000\r\n111"); + term.goto(Line(0), Column(0)); + + let hyperlink_foo = Hyperlink::new(Some("1"), "foo"); + let hyperlink_bar = Hyperlink::new(Some("2"), "bar"); + + // Create 2 hyperlinks on the first line. + term.set_hyperlink(Some(hyperlink_foo.clone())); + term.input('b'); + term.input('a'); + term.set_hyperlink(Some(hyperlink_bar.clone())); + term.input('r'); + term.set_hyperlink(Some(hyperlink_foo.clone())); + term.goto(Line(1), Column(0)); + + // Ditto for the second line. + term.set_hyperlink(Some(hyperlink_foo)); + term.input('b'); + term.input('a'); + term.set_hyperlink(Some(hyperlink_bar)); + term.input('r'); + term.set_hyperlink(None); + + let mut unique_hyperlinks = visible_unique_hyperlinks_iter(&term); + assert_eq!( + Some(Match::new(Point::new(Line(0), Column(0)), Point::new(Line(0), Column(1)))), + unique_hyperlinks.next() + ); + assert_eq!( + Some(Match::new(Point::new(Line(0), Column(2)), Point::new(Line(0), Column(2)))), + unique_hyperlinks.next() + ); + assert_eq!(None, unique_hyperlinks.next()); + } } diff --git a/alacritty_terminal/src/event.rs b/alacritty_terminal/src/event.rs index 9e5031a0..b592ce1b 100644 --- a/alacritty_terminal/src/event.rs +++ b/alacritty_terminal/src/event.rs @@ -99,6 +99,7 @@ pub trait EventListener { fn send_event(&self, _event: Event) {} } -/// Placeholder implementation for tests. -#[cfg(test)] -impl EventListener for () {} +/// Null sink for events. +pub struct VoidListener; + +impl EventListener for VoidListener {} diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs index 1f3269c2..5baa3df4 100644 --- a/alacritty_terminal/src/term/mod.rs +++ b/alacritty_terminal/src/term/mod.rs @@ -2089,6 +2089,7 @@ pub mod test { use unicode_width::UnicodeWidthChar; use crate::config::Config; + use crate::event::VoidListener; use crate::index::Column; #[derive(Serialize, Deserialize)] @@ -2136,7 +2137,7 @@ pub mod test { /// hello\n:)\r\ntest", /// ); /// ``` - pub fn mock_term(content: &str) -> Term<()> { + pub fn mock_term(content: &str) -> Term { let lines: Vec<&str> = content.split('\n').collect(); let num_cols = lines .iter() @@ -2146,7 +2147,7 @@ pub mod test { // Create terminal with the appropriate dimensions. let size = TermSize::new(num_cols, lines.len()); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Fill terminal with content. for (line, text) in lines.iter().enumerate() { @@ -2182,6 +2183,7 @@ mod tests { use crate::ansi::{self, CharsetIndex, Handler, StandardCharset}; use crate::config::Config; + use crate::event::VoidListener; use crate::grid::{Grid, Scroll}; use crate::index::{Column, Point, Side}; use crate::selection::{Selection, SelectionType}; @@ -2191,7 +2193,7 @@ mod tests { #[test] fn scroll_display_page_up() { let size = TermSize::new(5, 10); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Create 11 lines of scrollback. for _ in 0..20 { @@ -2217,7 +2219,7 @@ mod tests { #[test] fn scroll_display_page_down() { let size = TermSize::new(5, 10); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Create 11 lines of scrollback. for _ in 0..20 { @@ -2247,7 +2249,7 @@ mod tests { #[test] fn simple_selection_works() { let size = TermSize::new(5, 5); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); let grid = term.grid_mut(); for i in 0..4 { if i == 1 { @@ -2293,7 +2295,7 @@ mod tests { #[test] fn semantic_selection_works() { let size = TermSize::new(5, 3); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); let mut grid: Grid = Grid::new(3, 5, 0); for i in 0..5 { for j in 0..2 { @@ -2341,7 +2343,7 @@ mod tests { #[test] fn line_selection_works() { let size = TermSize::new(5, 1); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); let mut grid: Grid = Grid::new(1, 5, 0); for i in 0..5 { grid[Line(0)][Column(i)].c = 'a'; @@ -2362,7 +2364,7 @@ mod tests { #[test] fn block_selection_works() { let size = TermSize::new(5, 5); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); let grid = term.grid_mut(); for i in 1..4 { grid[Line(i)][Column(0)].c = '"'; @@ -2418,7 +2420,7 @@ mod tests { #[test] fn input_line_drawing_character() { let size = TermSize::new(7, 17); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); let cursor = Point::new(Line(0), Column(0)); term.configure_charset(CharsetIndex::G0, StandardCharset::SpecialCharacterAndLineDrawing); term.input('a'); @@ -2429,7 +2431,7 @@ mod tests { #[test] fn clearing_viewport_keeps_history_position() { let size = TermSize::new(10, 20); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Create 10 lines of scrollback. for _ in 0..29 { @@ -2450,7 +2452,7 @@ mod tests { #[test] fn clearing_viewport_with_vi_mode_keeps_history_position() { let size = TermSize::new(10, 20); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Create 10 lines of scrollback. for _ in 0..29 { @@ -2476,7 +2478,7 @@ mod tests { #[test] fn clearing_scrollback_resets_display_offset() { let size = TermSize::new(10, 20); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Create 10 lines of scrollback. for _ in 0..29 { @@ -2497,7 +2499,7 @@ mod tests { #[test] fn clearing_scrollback_sets_vi_cursor_into_viewport() { let size = TermSize::new(10, 20); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Create 10 lines of scrollback. for _ in 0..29 { @@ -2523,7 +2525,7 @@ mod tests { #[test] fn clear_saved_lines() { let size = TermSize::new(7, 17); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Add one line of scrollback. term.grid.scroll_up(&(Line(0)..Line(1)), 1); @@ -2545,7 +2547,7 @@ mod tests { #[test] fn vi_cursor_keep_pos_on_scrollback_buffer() { let size = TermSize::new(5, 10); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Create 11 lines of scrollback. for _ in 0..20 { @@ -2565,7 +2567,7 @@ mod tests { #[test] fn grow_lines_updates_active_cursor_pos() { let mut size = TermSize::new(100, 10); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Create 10 lines of scrollback. for _ in 0..19 { @@ -2585,7 +2587,7 @@ mod tests { #[test] fn grow_lines_updates_inactive_cursor_pos() { let mut size = TermSize::new(100, 10); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Create 10 lines of scrollback. for _ in 0..19 { @@ -2611,7 +2613,7 @@ mod tests { #[test] fn shrink_lines_updates_active_cursor_pos() { let mut size = TermSize::new(100, 10); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Create 10 lines of scrollback. for _ in 0..19 { @@ -2631,7 +2633,7 @@ mod tests { #[test] fn shrink_lines_updates_inactive_cursor_pos() { let mut size = TermSize::new(100, 10); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Create 10 lines of scrollback. for _ in 0..19 { @@ -2657,7 +2659,7 @@ mod tests { #[test] fn damage_public_usage() { let size = TermSize::new(10, 10); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Reset terminal for partial damage tests since it's initialized as fully damaged. term.reset_damage(); @@ -2750,7 +2752,7 @@ mod tests { #[test] fn damage_cursor_movements() { let size = TermSize::new(10, 10); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); let num_cols = term.columns(); // Reset terminal for partial damage tests since it's initialized as fully damaged. term.reset_damage(); @@ -2848,7 +2850,7 @@ mod tests { #[test] fn full_damage() { let size = TermSize::new(100, 10); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); assert!(term.damage.is_fully_damaged); for _ in 0..20 { @@ -2934,7 +2936,7 @@ mod tests { #[test] fn window_title() { let size = TermSize::new(7, 17); - let mut term = Term::new(&Config::default(), &size, ()); + let mut term = Term::new(&Config::default(), &size, VoidListener); // Title None by default. assert_eq!(term.title, None); diff --git a/alacritty_terminal/src/vi_mode.rs b/alacritty_terminal/src/vi_mode.rs index 7f1d40ee..0cdc6a69 100644 --- a/alacritty_terminal/src/vi_mode.rs +++ b/alacritty_terminal/src/vi_mode.rs @@ -380,13 +380,14 @@ mod tests { use crate::ansi::Handler; use crate::config::Config; + use crate::event::VoidListener; use crate::index::{Column, Line}; use crate::term::test::TermSize; use crate::term::Term; - fn term() -> Term<()> { + fn term() -> Term { let size = TermSize::new(20, 20); - Term::new(&Config::default(), &size, ()) + Term::new(&Config::default(), &size, VoidListener) } #[test] @@ -493,7 +494,7 @@ mod tests { assert_eq!(cursor.point, Point::new(Line(0), Column(0))); } - fn motion_semantic_term() -> Term<()> { + fn motion_semantic_term() -> Term { let mut term = term(); term.grid_mut()[Line(0)][Column(0)].c = 'x';