Skip to content

Commit 6653261

Browse files
the-mikedavispickfire
authored andcommitted
Improve keymap errors from command typos (helix-editor#3931)
* 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]>
1 parent ee14292 commit 6653261

File tree

1 file changed

+58
-2
lines changed

1 file changed

+58
-2
lines changed

helix-term/src/keymap.rs

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,14 +144,70 @@ impl DerefMut for KeyTrieNode {
144144
}
145145
}
146146

147-
#[derive(Debug, Clone, PartialEq, Deserialize)]
148-
#[serde(untagged)]
147+
#[derive(Debug, Clone, PartialEq)]
149148
pub enum KeyTrie {
150149
Leaf(MappableCommand),
151150
Sequence(Vec<MappableCommand>),
152151
Node(KeyTrieNode),
153152
}
154153

154+
impl<'de> Deserialize<'de> for KeyTrie {
155+
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
156+
where
157+
D: serde::Deserializer<'de>,
158+
{
159+
deserializer.deserialize_any(KeyTrieVisitor)
160+
}
161+
}
162+
163+
struct KeyTrieVisitor;
164+
165+
impl<'de> serde::de::Visitor<'de> for KeyTrieVisitor {
166+
type Value = KeyTrie;
167+
168+
fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
169+
write!(formatter, "a command, list of commands, or sub-keymap")
170+
}
171+
172+
fn visit_str<E>(self, command: &str) -> Result<Self::Value, E>
173+
where
174+
E: serde::de::Error,
175+
{
176+
command
177+
.parse::<MappableCommand>()
178+
.map(KeyTrie::Leaf)
179+
.map_err(E::custom)
180+
}
181+
182+
fn visit_seq<S>(self, mut seq: S) -> Result<Self::Value, S::Error>
183+
where
184+
S: serde::de::SeqAccess<'de>,
185+
{
186+
let mut commands = Vec::new();
187+
while let Some(command) = seq.next_element::<&str>()? {
188+
commands.push(
189+
command
190+
.parse::<MappableCommand>()
191+
.map_err(serde::de::Error::custom)?,
192+
)
193+
}
194+
Ok(KeyTrie::Sequence(commands))
195+
}
196+
197+
fn visit_map<M>(self, mut map: M) -> Result<Self::Value, M::Error>
198+
where
199+
M: serde::de::MapAccess<'de>,
200+
{
201+
let mut mapping = HashMap::new();
202+
let mut order = Vec::new();
203+
while let Some((key, value)) = map.next_entry::<KeyEvent, KeyTrie>()? {
204+
mapping.insert(key, value);
205+
order.push(key);
206+
}
207+
Ok(KeyTrie::Node(KeyTrieNode::new("", mapping, order)))
208+
}
209+
}
210+
155211
impl KeyTrie {
156212
pub fn node(&self) -> Option<&KeyTrieNode> {
157213
match *self {

0 commit comments

Comments
 (0)