feat: Gesture configuration in shortcuts config #63
Conversation
ryanabx
force-pushed
the
gestures
branch
7 times, most recently
from
23542a8 to
93f7fb1
Compare
|
Rebased on latest master. Can I get a review soon if there's any free time? |
This commit adds a Gesture abstraction, which can define gestures by the number of fingers used, and the direction the gesture is performed in. The abstraction is modeled after the Binding type, and the resulting Gestures type is similar to the Shortcuts type. This is the first step in implementing configurable touchpad gestures (pop-os/cosmic-comp#396) Signed-off-by: Ryan Brue <[email protected]>
| Direction::Down => "Down".to_string(), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
You can avoid allocating a string with
impl From<Direction> for &'static str {
fn from(direction: Direction) -> &'static str {
match self {
Direction::Left => "Left",
Direction::Right => "Right",
Direction::Up => "Up",
Direction::Down => "Down",
}
}
}Then you can use it like so:
<&'static str>::from(direction)
|
|
||
| impl Gesture { | ||
| /// Creates a new gesture from a number of fingers and a direction | ||
| pub fn new(fingers: impl Into<u32>, direction: impl Into<Direction>) -> Gesture { |
There was a problem hiding this comment.
I don't think there's a need for generic input parameters here. Use of constant types will prevent unnecessary monomorphization and inlining.
| self.to_string_in_place(&mut string); | ||
| string | ||
| } | ||
| } |
There was a problem hiding this comment.
Clippy recommends implementing std::fmt::Display instead, so that it can be used directly in a format arg without extra string allocations.
| } | ||
|
|
||
| impl FromStr for Gesture { | ||
| type Err = String; |
There was a problem hiding this comment.
Could use thiserror to make this a proper error type since this a library.
| Err(_) => { | ||
| return Err(format!("could not parse number of fingers")); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Rust got new syntax lately to handle this better
let Some(fingers) = value_iter.next() else {
return Err(Error::NoFingerValue);
};
let Ok(fingers) = u32::from_str(fingers) else {
return Err(Error::FingerNaN);
};Or
let fingers = u32::from_str(value_iter.next().ok_or(Error::NoFingerValue)?)
.map_err(|_| Error::FingerNaN)?;
| let mut gestures = context | ||
| .get::<Gestures>("default_gestures") | ||
| .unwrap_or_else(|why| { | ||
| tracing::error!("shortcuts defaults config error: {why:?}"); |
There was a problem hiding this comment.
Use if why.is_err() {} to check if the error is worth logging. Otherwise this will spam the logs if the config is not in use.
| /// A map of defined [Gesture]s and their triggerable [Action]s | ||
| #[derive(Clone, Debug, Default, PartialEq, Deserialize, Serialize)] | ||
| #[serde(transparent)] | ||
| pub struct Gestures(pub HashMap<Gesture, Action>); |
There was a problem hiding this comment.
BTreeMap might be more efficient since there's only a few values. I think HashMap becomes more efficient after ~1000 values.
This commit adds a Gesture abstraction, which can define gestures by the number of fingers used, and the direction the gesture is performed in.
The abstraction is modeled after the Binding type, and the resulting Gestures type is similar to the Shortcuts type.
This is the first step in implementing configurable touchpad gestures (pop-os/cosmic-comp#396)
Let me know what needs to be changed!