Force exact modifiers match for mouse bindings

Fixes #3152.
This commit is contained in:
Christian Duerr 2020-01-10 00:44:41 +00:00 committed by GitHub
parent 3fb631b91c
commit dd1413eb4d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 55 additions and 64 deletions

View File

@ -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

View File

@ -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`:

View File

@ -81,20 +81,14 @@ impl Default for MouseBinding {
impl<T: Eq> Binding<T> {
#[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<T: ?Sized = Self> {
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<E>(self, value: &str) -> Result<ModeWrapper, E>
@ -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));
}
}

View File

@ -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;

View File

@ -91,20 +91,20 @@ pub trait ActionContext<T: EventListener> {
}
trait Execute<T: EventListener> {
fn execute<A: ActionContext<T>>(&self, ctx: &mut A, mouse_mode: bool);
fn execute<A: ActionContext<T>>(&self, ctx: &mut A);
}
impl<T, U: EventListener> Execute<U> for Binding<T> {
/// Execute the action associate with this binding
#[inline]
fn execute<A: ActionContext<U>>(&self, ctx: &mut A, mouse_mode: bool) {
self.action.execute(ctx, mouse_mode)
fn execute<A: ActionContext<U>>(&self, ctx: &mut A) {
self.action.execute(ctx)
}
}
impl<T: EventListener> Execute<T> for Action {
#[inline]
fn execute<A: ActionContext<T>>(&self, ctx: &mut A, mouse_mode: bool) {
fn execute<A: ActionContext<T>>(&self, ctx: &mut A) {
match *self {
Action::Esc(ref s) => {
ctx.clear_selection();
@ -119,11 +119,8 @@ impl<T: EventListener> Execute<T> 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<T>> 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<T>> 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<T>> 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));
}
}
}

View File

@ -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<Url> {
// 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;