Skip to content

Commit

Permalink
Fix hyperlinks not being keyboard actionable
Browse files Browse the repository at this point in the history 
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 <contact@christianduerr.com>
  • Loading branch information
@kchibisov @chrisduerr
kchibisov and chrisduerr committed Jul 11, 2022
1 parent e09ff0b commit 40bbdce
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 32 deletions.
44 changes: 41 additions & 3 deletions alacritty/src/display/hint.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<T>(term: &Term<T>) -> impl Iterator<Item = Match> + '_ {
pub fn visible_unique_hyperlinks_iter<T>(term: &Term<T>) -> impl Iterator<Item = Match> + '_ {
let mut display_iter = term.grid().display_iter().peekable();

// Avoid creating hints for the same hyperlinks, but from a different places.
Expand All @@ -308,7 +308,7 @@ pub fn visible_unique_hyperlink_iter<T>(term: &Term<T>) -> impl Iterator<Item =
// Find the start of the next unique hyperlink.
let (cell, hyperlink) = display_iter.find_map(|cell| {
let hyperlink = cell.hyperlink()?;
unique_hyperlinks.contains(&hyperlink).then(|| {
(!unique_hyperlinks.contains(&hyperlink)).then(|| {
unique_hyperlinks.insert(hyperlink.clone());
(cell, hyperlink)
})
Expand Down Expand Up @@ -582,6 +582,7 @@ impl<'a, T> 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;

Expand Down Expand Up @@ -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());
}
}
7 changes: 4 additions & 3 deletions alacritty_terminal/src/event.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
48 changes: 25 additions & 23 deletions alacritty_terminal/src/term/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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<VoidListener> {
let lines: Vec<&str> = content.split('\n').collect();
let num_cols = lines
.iter()
Expand All @@ -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() {
Expand Down Expand Up @@ -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};
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<Cell> = Grid::new(3, 5, 0);
for i in 0..5 {
for j in 0..2 {
Expand Down Expand Up @@ -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<Cell> = Grid::new(1, 5, 0);
for i in 0..5 {
grid[Line(0)][Column(i)].c = 'a';
Expand All @@ -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 = '"';
Expand Down Expand Up @@ -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');
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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);
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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();

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 4 additions & 3 deletions alacritty_terminal/src/vi_mode.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<VoidListener> {
let size = TermSize::new(20, 20);
Term::new(&Config::default(), &size, ())
Term::new(&Config::default(), &size, VoidListener)
}

#[test]
Expand Down Expand Up @@ -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<VoidListener> {
let mut term = term();

term.grid_mut()[Line(0)][Column(0)].c = 'x';
Expand Down

0 comments on commit 40bbdce

Please sign in to comment.