Skip to content

Commit 0f9af3f

Browse files
committed
Propogate value errors to user
This should provide more meaningful errors to the user and is somewhat closer to how I would expect a parser to work in a situation like this (ie we should ensure `cut` errors get propagated). The main risk is if there is any error recovery meant to be going on.
1 parent 5366f79 commit 0f9af3f

File tree

2 files changed

+16
-12
lines changed

2 files changed

+16
-12
lines changed

gix-config/src/parse/nom/mod.rs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,7 @@ fn section<'i>(
121121
dispatch(Event::Newline(Cow::Borrowed(v.as_bstr())));
122122
}
123123

124-
let before_key = i.checkpoint();
125-
if key_value_pair(i, node, dispatch).is_err() {
126-
i.reset(before_key);
127-
}
124+
key_value_pair(i, node, dispatch)?;
128125

129126
if let Some(comment) = opt(comment).parse_next(i)? {
130127
dispatch(Event::Comment(comment));
@@ -216,16 +213,18 @@ fn key_value_pair<'i>(
216213
dispatch: &mut impl FnMut(Event<'i>),
217214
) -> PResult<(), NomError<&'i [u8]>> {
218215
*node = ParseNode::Name;
219-
let name = config_name.parse_next(i)?;
216+
if let Some(name) = opt(config_name).parse_next(i)? {
217+
dispatch(Event::SectionKey(section::Key(Cow::Borrowed(name))));
220218

221-
dispatch(Event::SectionKey(section::Key(Cow::Borrowed(name))));
219+
if let Some(whitespace) = opt(take_spaces1).parse_next(i)? {
220+
dispatch(Event::Whitespace(Cow::Borrowed(whitespace)));
221+
}
222222

223-
if let Some(whitespace) = opt(take_spaces1).parse_next(i)? {
224-
dispatch(Event::Whitespace(Cow::Borrowed(whitespace)));
223+
*node = ParseNode::Value;
224+
config_value(i, dispatch)
225+
} else {
226+
Ok(())
225227
}
226-
227-
*node = ParseNode::Value;
228-
config_value(i, dispatch)
229228
}
230229

231230
/// Parses the config name of a config pair. Assumes the input has already been

gix-config/src/parse/nom/tests.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,12 @@ mod key_value_pair {
808808
fn nonascii_is_allowed_for_values_but_not_for_keys() {
809809
let mut node = ParseNode::SectionHeader;
810810
let mut vec = Default::default();
811-
assert!(key_value("你好".as_bytes(), &mut node, &mut vec).is_err());
811+
// Verifying `is_ok` because bad keys get ignored
812+
assert!(key_value("你好".as_bytes(), &mut node, &mut vec).is_ok());
813+
assert_eq!(vec, into_events(vec![]));
814+
815+
let mut node = ParseNode::SectionHeader;
816+
let mut vec = Default::default();
812817
assert!(key_value("a = 你好 ".as_bytes(), &mut node, &mut vec).is_ok());
813818
assert_eq!(
814819
vec,

0 commit comments

Comments
 (0)