Skip to content

Commit

Permalink
Improve keymap errors from command typos (#3931)
Browse files Browse the repository at this point in the history
* Improve keymap errors from command typos

Currently, opening helix with a config containing a bad command mapping
fails with a cryptic error. For example, say we have a config (bad.toml)
with a command name that doesn't exist:

    [keys.normal]
    b = "buffer_close" # should be ":buffer-close"

When we `hx -c bad.toml`, we get...

> Bad config: data did not match any variant of untagged enum KeyTrie for key `keys.normal` at line 1 column 1
> Press <ENTER> to continue with default config

This is because of the way that Serde tries to deserialize untagged
enums such as `helix_term::keymap::KeyTrie`. From the Serde docs[^1]:

> Serde will try to match the data against each variant in order and the
> first one that deserializes successfully is the one returned.

`MappableCommand::deserialize` fails (returns an Err variant) when a
command does not exist. Serde interprets this as the `KeyTrie::Leaf`
variant failing to match and declares that the input data doesn't
"match any variant of untagged enum KeyTrie."

Luckily the variants of KeyTrie are orthogonal in structure: we can tell
them apart by the type hints from a `serde::de::Visitor`. This change
uses a custom Deserialize implementation along with a Visitor that
discerns which variant of the KeyTrie applies. With this change, the
above failure becomes:

> Bad config: No command named 'buffer_close' for key `keys.normal.b` at line 2 column 5
> Press <ENTER> to continue with default config

We also provide more explicit information about the expectations on
the field. A config with an unexpected type produces a message with
that information and the expectation:

    [keys.normal]
    b = 1

> Bad config: invalid type: integer `1`, expected a command, list of commands, or sub-keymap for key `keys.normal.b` at line 2 column 5
> Press <ENTER> to continue with default config

[^1]: https://serde.rs/enum-representations.html#untagged

* Update helix-term/src/keymap.rs

Co-authored-by: Ivan Tham <[email protected]>

Co-authored-by: Ivan Tham <[email protected]>
  • Loading branch information
the-mikedavis and pickfire authored Sep 22, 2022
1 parent 1dd1476 commit 6e168b5
Showing 1 changed file with 58 additions and 2 deletions.
60 changes: 58 additions & 2 deletions helix-term/src/keymap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,70 @@ impl DerefMut for KeyTrieNode {
}
}

#[derive(Debug, Clone, PartialEq, Deserialize)]
#[serde(untagged)]
#[derive(Debug, Clone, PartialEq)]
pub enum KeyTrie {
Leaf(MappableCommand),
Sequence(Vec<MappableCommand>),
Node(KeyTrieNode),
}

impl<'de> Deserialize<'de> for KeyTrie {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
deserializer.deserialize_any(KeyTrieVisitor)
}
}

struct KeyTrieVisitor;

impl<'de> serde::de::Visitor<'de> for KeyTrieVisitor {
type Value = KeyTrie;

fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(formatter, "a command, list of commands, or sub-keymap")
}

fn visit_str<E>(self, command: &str) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
command
.parse::<MappableCommand>()
.map(KeyTrie::Leaf)
.map_err(E::custom)
}

fn visit_seq<S>(self, mut seq: S) -> Result<Self::Value, S::Error>
where
S: serde::de::SeqAccess<'de>,
{
let mut commands = Vec::new();
while let Some(command) = seq.next_element::<&str>()? {
commands.push(
command
.parse::<MappableCommand>()
.map_err(serde::de::Error::custom)?,
)
}
Ok(KeyTrie::Sequence(commands))
}

fn visit_map<M>(self, mut map: M) -> Result<Self::Value, M::Error>
where
M: serde::de::MapAccess<'de>,
{
let mut mapping = HashMap::new();
let mut order = Vec::new();
while let Some((key, value)) = map.next_entry::<KeyEvent, KeyTrie>()? {
mapping.insert(key, value);
order.push(key);
}
Ok(KeyTrie::Node(KeyTrieNode::new("", mapping, order)))
}
}

impl KeyTrie {
pub fn node(&self) -> Option<&KeyTrieNode> {
match *self {
Expand Down

0 comments on commit 6e168b5

Please sign in to comment.