From dd1413eb4d4f8cb170458155e5156e569c14130e Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Fri, 10 Jan 2020 00:44:41 +0000 Subject: [PATCH] Force exact modifiers match for mouse bindings Fixes #3152. --- CHANGELOG.md | 4 +++ alacritty.yml | 3 ++ alacritty/src/config/bindings.rs | 55 ++++++++++---------------------- alacritty/src/config/mod.rs | 2 +- alacritty/src/input.rs | 44 ++++++++++++------------- alacritty/src/url.rs | 11 +++++-- 6 files changed, 55 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7387fcc..423d1a98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## 0.4.2-dev +### Changed + +- Pressing additional modifiers for mouse bindings will no longer trigger them + ### Fixed - Incorrect default config path in `--help` on Windows and macOS diff --git a/alacritty.yml b/alacritty.yml index f30b5707..63d1927b 100644 --- a/alacritty.yml +++ b/alacritty.yml @@ -413,6 +413,9 @@ # Mouse bindings are specified as a list of objects, much like the key # bindings further below. # +# To trigger mouse bindings when an application running within Alacritty captures the mouse, the +# `Shift` modifier is automatically added as a requirement. +# # Each mouse binding will specify a: # # - `mouse`: diff --git a/alacritty/src/config/bindings.rs b/alacritty/src/config/bindings.rs index abdb1727..1a9f95af 100644 --- a/alacritty/src/config/bindings.rs +++ b/alacritty/src/config/bindings.rs @@ -81,20 +81,14 @@ impl Default for MouseBinding { impl Binding { #[inline] - pub fn is_triggered_by( - &self, - mode: TermMode, - mods: ModifiersState, - input: &T, - relaxed: bool, - ) -> bool { + pub fn is_triggered_by(&self, mode: TermMode, mods: ModifiersState, input: &T) -> bool { // Check input first since bindings are stored in one big list. This is // the most likely item to fail so prioritizing it here allows more // checks to be short circuited. self.trigger == *input && mode.contains(self.mode) && !mode.intersects(self.notmode) - && (self.mods == mods || (relaxed && self.mods.relaxed_eq(mods))) + && (self.mods == mods) } #[inline] @@ -207,18 +201,6 @@ impl From<&'static str> for Action { } } -pub trait RelaxedEq { - fn relaxed_eq(&self, other: T) -> bool; -} - -impl RelaxedEq for ModifiersState { - // Make sure that modifiers in the config are always present, - // but ignore surplus modifiers. - fn relaxed_eq(&self, other: Self) -> bool { - !*self | other == ModifiersState::all() - } -} - macro_rules! bindings { ( KeyBinding; @@ -507,7 +489,9 @@ impl<'a> Deserialize<'a> for ModeWrapper { type Value = ModeWrapper; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("Combination of AppCursor | AppKeypad, possibly with negation (~)") + f.write_str( + "Combination of AppCursor | AppKeypad | Alt, possibly with negation (~)", + ) } fn visit_str(self, value: &str) -> Result @@ -522,8 +506,8 @@ impl<'a> Deserialize<'a> for ModeWrapper { "~appcursor" => res.not_mode |= TermMode::APP_CURSOR, "appkeypad" => res.mode |= TermMode::APP_KEYPAD, "~appkeypad" => res.not_mode |= TermMode::APP_KEYPAD, - "~alt" => res.not_mode |= TermMode::ALT_SCREEN, "alt" => res.mode |= TermMode::ALT_SCREEN, + "~alt" => res.not_mode |= TermMode::ALT_SCREEN, _ => error!(target: LOG_TARGET_CONFIG, "Unknown mode {:?}", modifier), } } @@ -1040,8 +1024,8 @@ mod test { let mods = binding.mods; let mode = binding.mode; - assert!(binding.is_triggered_by(mode, mods, &13, true)); - assert!(!binding.is_triggered_by(mode, mods, &32, true)); + assert!(binding.is_triggered_by(mode, mods, &13)); + assert!(!binding.is_triggered_by(mode, mods, &32)); } #[test] @@ -1055,14 +1039,9 @@ mod test { let t = binding.trigger; let mode = binding.mode; - assert!(binding.is_triggered_by(mode, binding.mods, &t, true)); - assert!(binding.is_triggered_by(mode, binding.mods, &t, false)); - - assert!(binding.is_triggered_by(mode, superset_mods, &t, true)); - assert!(!binding.is_triggered_by(mode, superset_mods, &t, false)); - - assert!(!binding.is_triggered_by(mode, subset_mods, &t, true)); - assert!(!binding.is_triggered_by(mode, subset_mods, &t, false)); + assert!(binding.is_triggered_by(mode, binding.mods, &t)); + assert!(!binding.is_triggered_by(mode, superset_mods, &t)); + assert!(!binding.is_triggered_by(mode, subset_mods, &t)); } #[test] @@ -1073,9 +1052,9 @@ mod test { let t = binding.trigger; let mods = binding.mods; - assert!(!binding.is_triggered_by(TermMode::INSERT, mods, &t, true)); - assert!(binding.is_triggered_by(TermMode::ALT_SCREEN, mods, &t, true)); - assert!(binding.is_triggered_by(TermMode::ALT_SCREEN | TermMode::INSERT, mods, &t, true)); + assert!(!binding.is_triggered_by(TermMode::INSERT, mods, &t)); + assert!(binding.is_triggered_by(TermMode::ALT_SCREEN, mods, &t)); + assert!(binding.is_triggered_by(TermMode::ALT_SCREEN | TermMode::INSERT, mods, &t)); } #[test] @@ -1086,8 +1065,8 @@ mod test { let t = binding.trigger; let mods = binding.mods; - assert!(binding.is_triggered_by(TermMode::INSERT, mods, &t, true)); - assert!(!binding.is_triggered_by(TermMode::ALT_SCREEN, mods, &t, true)); - assert!(!binding.is_triggered_by(TermMode::ALT_SCREEN | TermMode::INSERT, mods, &t, true)); + assert!(binding.is_triggered_by(TermMode::INSERT, mods, &t)); + assert!(!binding.is_triggered_by(TermMode::ALT_SCREEN, mods, &t)); + assert!(!binding.is_triggered_by(TermMode::ALT_SCREEN | TermMode::INSERT, mods, &t)); } } diff --git a/alacritty/src/config/mod.rs b/alacritty/src/config/mod.rs index 91ca6936..e489f5fb 100644 --- a/alacritty/src/config/mod.rs +++ b/alacritty/src/config/mod.rs @@ -18,7 +18,7 @@ pub mod monitor; mod mouse; mod ui_config; -pub use crate::config::bindings::{Action, Binding, Key, RelaxedEq}; +pub use crate::config::bindings::{Action, Binding, Key}; #[cfg(test)] pub use crate::config::mouse::{ClickHandler, Mouse}; use crate::config::ui_config::UIConfig; diff --git a/alacritty/src/input.rs b/alacritty/src/input.rs index 39f54328..c15c66d6 100644 --- a/alacritty/src/input.rs +++ b/alacritty/src/input.rs @@ -91,20 +91,20 @@ pub trait ActionContext { } trait Execute { - fn execute>(&self, ctx: &mut A, mouse_mode: bool); + fn execute>(&self, ctx: &mut A); } impl Execute for Binding { /// Execute the action associate with this binding #[inline] - fn execute>(&self, ctx: &mut A, mouse_mode: bool) { - self.action.execute(ctx, mouse_mode) + fn execute>(&self, ctx: &mut A) { + self.action.execute(ctx) } } impl Execute for Action { #[inline] - fn execute>(&self, ctx: &mut A, mouse_mode: bool) { + fn execute>(&self, ctx: &mut A) { match *self { Action::Esc(ref s) => { ctx.clear_selection(); @@ -119,11 +119,8 @@ impl Execute for Action { paste(ctx, &text); }, Action::PasteSelection => { - // Only paste if mouse events are not captured by an application - if !mouse_mode { - let text = ctx.terminal_mut().clipboard().load(ClipboardType::Selection); - paste(ctx, &text); - } + let text = ctx.terminal_mut().clipboard().load(ClipboardType::Selection); + paste(ctx, &text); }, Action::Command(ref program, ref args) => { trace!("Running command {} with args {:?}", program, args); @@ -648,10 +645,10 @@ impl<'a, T: EventListener, A: ActionContext> Processor<'a, T, A> { /// The provided mode, mods, and key must match what is allowed by a binding /// for its action to be executed. fn process_key_bindings(&mut self, input: KeyboardInput) { + let mods = *self.ctx.modifiers(); let mut suppress_chars = None; for i in 0..self.ctx.config().ui_config.key_bindings.len() { - let mods = *self.ctx.modifiers(); let binding = &self.ctx.config().ui_config.key_bindings[i]; let key = match (binding.trigger, input.virtual_keycode) { @@ -660,10 +657,10 @@ impl<'a, T: EventListener, A: ActionContext> Processor<'a, T, A> { _ => continue, }; - if binding.is_triggered_by(*self.ctx.terminal().mode(), mods, &key, false) { + if binding.is_triggered_by(*self.ctx.terminal().mode(), mods, &key) { // Binding was triggered; run the action let binding = binding.clone(); - binding.execute(&mut self.ctx, false); + binding.execute(&mut self.ctx); // Don't suppress when there has been a `ReceiveChar` action *suppress_chars.get_or_insert(true) &= binding.action != Action::ReceiveChar; @@ -679,17 +676,20 @@ impl<'a, T: EventListener, A: ActionContext> Processor<'a, T, A> { /// The provided mode, mods, and key must match what is allowed by a binding /// for its action to be executed. fn process_mouse_bindings(&mut self, button: MouseButton) { + let mods = *self.ctx.modifiers(); + let mode = *self.ctx.terminal().mode(); + let mouse_mode = mode.intersects(TermMode::MOUSE_MODE); + for i in 0..self.ctx.config().ui_config.mouse_bindings.len() { - let mods = *self.ctx.modifiers(); - let binding = &self.ctx.config().ui_config.mouse_bindings[i]; + let mut binding = self.ctx.config().ui_config.mouse_bindings[i].clone(); - if binding.is_triggered_by(*self.ctx.terminal().mode(), mods, &button, true) { - // binding was triggered; run the action - let mouse_mode_active = - !mods.shift() && self.ctx.terminal().mode().intersects(TermMode::MOUSE_MODE); + // Require shift for all modifiers when mouse mode is active + if mouse_mode { + binding.mods |= ModifiersState::SHIFT; + } - let binding = binding.clone(); - binding.execute(&mut self.ctx, mouse_mode_active); + if binding.is_triggered_by(mode, mods, &button) { + binding.execute(&mut self.ctx); } } } @@ -1003,9 +1003,9 @@ mod tests { #[test] fn $name() { if $triggers { - assert!($binding.is_triggered_by($mode, $mods, &KEY, false)); + assert!($binding.is_triggered_by($mode, $mods, &KEY)); } else { - assert!(!$binding.is_triggered_by($mode, $mods, &KEY, false)); + assert!(!$binding.is_triggered_by($mode, $mods, &KEY)); } } } diff --git a/alacritty/src/url.rs b/alacritty/src/url.rs index b32a6812..4c5aed2a 100644 --- a/alacritty/src/url.rs +++ b/alacritty/src/url.rs @@ -11,7 +11,7 @@ use alacritty_terminal::term::cell::Flags; use alacritty_terminal::term::color::Rgb; use alacritty_terminal::term::{RenderableCell, RenderableCellContent, SizeInfo}; -use crate::config::{Config, RelaxedEq}; +use crate::config::Config; use crate::event::Mouse; use crate::renderer::rects::{RenderLine, RenderRect}; @@ -155,12 +155,17 @@ impl Urls { mouse_mode: bool, selection: bool, ) -> Option { + // Require additional shift in mouse mode + let mut required_mods = config.ui_config.mouse.url.mods(); + if mouse_mode { + required_mods |= ModifiersState::SHIFT; + } + // Make sure all prerequisites for highlighting are met if selection - || (mouse_mode && !mods.shift()) || !mouse.inside_grid || config.ui_config.mouse.url.launcher.is_none() - || !config.ui_config.mouse.url.mods().relaxed_eq(mods) + || required_mods != mods || mouse.left_button_state == ElementState::Pressed { return None;