Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor touchups in keymap.rs #7167

Closed
wants to merge 5 commits into from
Closed

Minor touchups in keymap.rs #7167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
86a403d
keymap: Derive `Default` for KeyTrieNode
gibbz00 May 31, 2023
60d20c4
keymap: Rename KeyTrie::Leaf -> KeyTrie::MapppableCommand
gibbz00 May 31, 2023
531fe2e
Make `Keymap` a tuple struct.
gibbz00 May 31, 2023
9060b9f
`helix_term::keymap`: Remove one-liner solely used for a test.
gibbz00 May 29, 2023
70c6565
Remove superfluous command description pruning for keymap infobox:
gibbz00 May 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 51 additions & 62 deletions helix-term/src/keymap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::{
pub use default::default;
use macros::key;

#[derive(Debug, Clone)]
#[derive(Debug, Clone, Default)]
pub struct KeyTrieNode {
/// A label for keys coming under this node, like "Goto mode"
name: String,
Expand Down Expand Up @@ -80,7 +80,7 @@ impl KeyTrieNode {
let mut body: Vec<(&str, BTreeSet<KeyEvent>)> = Vec::with_capacity(self.len());
for (&key, trie) in self.iter() {
let desc = match trie {
KeyTrie::Leaf(cmd) => {
KeyTrie::MappableCommand(cmd) => {
if cmd.name() == "no_op" {
continue;
}
Expand All @@ -102,25 +102,8 @@ impl KeyTrieNode {
.position(|&k| k == *keys.iter().next().unwrap())
.unwrap()
});
let prefix = format!("{} ", self.name());
if body.iter().all(|(desc, _)| desc.starts_with(&prefix)) {
body = body
.into_iter()
.map(|(desc, keys)| (desc.strip_prefix(&prefix).unwrap(), keys))
.collect();
}
Info::from_keymap(self.name(), body)
}
/// Get a reference to the key trie node's order.
pub fn order(&self) -> &[KeyEvent] {
self.order.as_slice()
}
}

impl Default for KeyTrieNode {
fn default() -> Self {
Self::new("", HashMap::new(), Vec::new())
}
}

impl PartialEq for KeyTrieNode {
Expand All @@ -145,7 +128,7 @@ impl DerefMut for KeyTrieNode {

#[derive(Debug, Clone, PartialEq)]
pub enum KeyTrie {
Leaf(MappableCommand),
MappableCommand(MappableCommand),
Sequence(Vec<MappableCommand>),
Node(KeyTrieNode),
}
Expand Down Expand Up @@ -174,7 +157,7 @@ impl<'de> serde::de::Visitor<'de> for KeyTrieVisitor {
{
command
.parse::<MappableCommand>()
.map(KeyTrie::Leaf)
.map(KeyTrie::MappableCommand)
.map_err(E::custom)
}

Expand Down Expand Up @@ -211,14 +194,14 @@ impl KeyTrie {
pub fn node(&self) -> Option<&KeyTrieNode> {
match *self {
KeyTrie::Node(ref node) => Some(node),
KeyTrie::Leaf(_) | KeyTrie::Sequence(_) => None,
KeyTrie::MappableCommand(_) | KeyTrie::Sequence(_) => None,
}
}

pub fn node_mut(&mut self) -> Option<&mut KeyTrieNode> {
match *self {
KeyTrie::Node(ref mut node) => Some(node),
KeyTrie::Leaf(_) | KeyTrie::Sequence(_) => None,
KeyTrie::MappableCommand(_) | KeyTrie::Sequence(_) => None,
}
}

Expand All @@ -235,7 +218,7 @@ impl KeyTrie {
trie = match trie {
KeyTrie::Node(map) => map.get(key),
// leaf encountered while keys left to process
KeyTrie::Leaf(_) | KeyTrie::Sequence(_) => None,
KeyTrie::MappableCommand(_) | KeyTrie::Sequence(_) => None,
}?
}
Some(trie)
Expand All @@ -258,24 +241,21 @@ pub enum KeymapResult {

#[derive(Debug, Clone, PartialEq, Deserialize)]
#[serde(transparent)]
pub struct Keymap {
/// Always a Node
root: KeyTrie,
}
pub struct Keymap(KeyTrie);

/// A map of command names to keybinds that will execute the command.
pub type ReverseKeymap = HashMap<String, Vec<Vec<KeyEvent>>>;

impl Keymap {
pub fn new(root: KeyTrie) -> Self {
Keymap { root }
Keymap(root)
}

pub fn reverse_map(&self) -> ReverseKeymap {
// recursively visit all nodes in keymap
fn map_node(cmd_map: &mut ReverseKeymap, node: &KeyTrie, keys: &mut Vec<KeyEvent>) {
match node {
KeyTrie::Leaf(cmd) => match cmd {
KeyTrie::MappableCommand(cmd) => match cmd {
MappableCommand::Typable { name, .. } => {
cmd_map.entry(name.into()).or_default().push(keys.clone())
}
Expand All @@ -296,30 +276,28 @@ impl Keymap {
}

let mut res = HashMap::new();
map_node(&mut res, &self.root, &mut Vec::new());
map_node(&mut res, &self.0, &mut Vec::new());
res
}

pub fn root(&self) -> &KeyTrie {
&self.root
}

pub fn merge(&mut self, other: Self) {
self.root.merge_nodes(other.root);
}
}

impl Deref for Keymap {
type Target = KeyTrieNode;
type Target = KeyTrie;

fn deref(&self) -> &Self::Target {
self.root.node().unwrap()
&self.0
}
}

impl DerefMut for Keymap {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

impl Default for Keymap {
fn default() -> Self {
Self::new(KeyTrie::Node(KeyTrieNode::default()))
Self(KeyTrie::Node(KeyTrieNode::default()))
}
}

Expand Down Expand Up @@ -373,11 +351,11 @@ impl Keymaps {
let first = self.state.get(0).unwrap_or(&key);
let trie_node = match self.sticky {
Some(ref trie) => Cow::Owned(KeyTrie::Node(trie.clone())),
None => Cow::Borrowed(&keymap.root),
None => Cow::Borrowed(&keymap.0),
};

let trie = match trie_node.search(&[*first]) {
Some(KeyTrie::Leaf(ref cmd)) => {
Some(KeyTrie::MappableCommand(ref cmd)) => {
return KeymapResult::Matched(cmd.clone());
}
Some(KeyTrie::Sequence(ref cmds)) => {
Expand All @@ -396,7 +374,7 @@ impl Keymaps {
}
KeymapResult::Pending(map.clone())
}
Some(KeyTrie::Leaf(cmd)) => {
Some(KeyTrie::MappableCommand(cmd)) => {
self.state.clear();
KeymapResult::Matched(cmd.clone())
}
Expand All @@ -418,7 +396,7 @@ impl Default for Keymaps {
/// Merge default config keys with user overwritten keys for custom user config.
pub fn merge_keys(dst: &mut HashMap<Mode, Keymap>, mut delta: HashMap<Mode, Keymap>) {
for (mode, keys) in dst {
keys.merge(delta.remove(mode).unwrap_or_default())
keys.merge_nodes(delta.remove(mode).unwrap_or_default().0)
}
}

Expand Down Expand Up @@ -484,25 +462,39 @@ mod tests {
let keymap = merged_keyamp.get_mut(&Mode::Normal).unwrap();
// Assumes that `g` is a node in default keymap
assert_eq!(
keymap.root().search(&[key!('g'), key!('$')]).unwrap(),
&KeyTrie::Leaf(MappableCommand::goto_line_end),
keymap.search(&[key!('g'), key!('$')]).unwrap(),
&KeyTrie::MappableCommand(MappableCommand::goto_line_end),
"Leaf should be present in merged subnode"
);
// Assumes that `gg` is in default keymap
assert_eq!(
keymap.root().search(&[key!('g'), key!('g')]).unwrap(),
&KeyTrie::Leaf(MappableCommand::delete_char_forward),
keymap.search(&[key!('g'), key!('g')]).unwrap(),
&KeyTrie::MappableCommand(MappableCommand::delete_char_forward),
"Leaf should replace old leaf in merged subnode"
);
// Assumes that `ge` is in default keymap
assert_eq!(
keymap.root().search(&[key!('g'), key!('e')]).unwrap(),
&KeyTrie::Leaf(MappableCommand::goto_last_line),
keymap.search(&[key!('g'), key!('e')]).unwrap(),
&KeyTrie::MappableCommand(MappableCommand::goto_last_line),
"Old leaves in subnode should be present in merged node"
);

assert!(merged_keyamp.get(&Mode::Normal).unwrap().len() > 1);
assert!(merged_keyamp.get(&Mode::Insert).unwrap().len() > 0);
assert!(
merged_keyamp
.get(&Mode::Normal)
.and_then(|key_trie| key_trie.node())
.unwrap()
.len()
> 1
);
assert!(
merged_keyamp
.get(&Mode::Insert)
.and_then(|key_trie| key_trie.node())
.unwrap()
.len()
> 0
);
}

#[test]
Expand All @@ -525,22 +517,19 @@ mod tests {
let keymap = merged_keyamp.get_mut(&Mode::Normal).unwrap();
// Make sure mapping works
assert_eq!(
keymap
.root()
.search(&[key!(' '), key!('s'), key!('v')])
.unwrap(),
&KeyTrie::Leaf(MappableCommand::vsplit),
keymap.search(&[key!(' '), key!('s'), key!('v')]).unwrap(),
&KeyTrie::MappableCommand(MappableCommand::vsplit),
"Leaf should be present in merged subnode"
);
// Make sure an order was set during merge
let node = keymap.root().search(&[crate::key!(' ')]).unwrap();
assert!(!node.node().unwrap().order().is_empty())
let node = keymap.search(&[crate::key!(' ')]).unwrap();
assert!(!node.node().unwrap().order.as_slice().is_empty())
}

#[test]
fn aliased_modes_are_same_in_default_keymap() {
let keymaps = Keymaps::default().map();
let root = keymaps.get(&Mode::Normal).unwrap().root();
let root = keymaps.get(&Mode::Normal).unwrap();
assert_eq!(
root.search(&[key!(' '), key!('w')]).unwrap(),
root.search(&["C-w".parse::<KeyEvent>().unwrap()]).unwrap(),
Expand Down
2 changes: 1 addition & 1 deletion helix-term/src/keymap/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ macro_rules! alt {
#[macro_export]
macro_rules! keymap {
(@trie $cmd:ident) => {
$crate::keymap::KeyTrie::Leaf($crate::commands::MappableCommand::$cmd)
$crate::keymap::KeyTrie::MappableCommand($crate::commands::MappableCommand::$cmd)
};

(@trie
Expand Down