use simpler GrmtoolsSectionValue throughout - #666
Conversation
| impl<'input> GrmtoolsSectionParser<'input> { | ||
| fn parse_setting(&'_ self, mut i: usize) -> Result<(Setting<Span>, usize), HeaderError<Span>> { | ||
| fn parse_setting( | ||
| &'_ self, |
There was a problem hiding this comment.
Just noticing that now a few of the functions like parse_setting and parse_namespaced which
historically made sense since they previously returned Setting and Namespaced probably need better names like parse_rustlike for parse_namespaced..
Alas I don't have a good idea for parse_setting, which is to the right hand side of the : in value: setting.
| member: ("None".to_string(), from_loc.clone()), | ||
| })), | ||
| RecoveryKind::CPCTPlus => { | ||
| GrmtoolsSectionValue::RustLike("RecoveryKind::CPCTPlus".to_string(), from_loc) |
There was a problem hiding this comment.
I think these can probably be simplified by using the ToTokens/implementations to produce the string
rather than these hard coded values.
Ditto for elsewhere.
There was a problem hiding this comment.
Alas, that doesn't seem to work (it emits a fully qualified path like :: lrpar :: RecoveryKind :: CPCTPlus)
There was a problem hiding this comment.
The best I could come up with to simplify these further was using Debug 3129318
2 participants
This is an experiment to see if we can do away with
Value<T>and all it's friendsSetting,Namespaced, etc.Replacing it with the much simpler
GrmtoolsSectionValue<T>. This can lead to slightly worse error messages.e.g. because we're checking
YaccKindas a whole includingYaccOriginalActionKind. But I think it is pretty much limited to that. We could perhaps do better, by attempting to parse theRustLike(string)rather than the simple matching I've done here.Sadly this patch has ended up kind of redoing some of the case-insensitivity migration in #665
This came up because when experimenting with integrading the lookup methods with the used value checking.
This involved changing the value owned by the
GrammarASTfrom aHashMap<GrmtoolsSectionValue>to theHeader<T>. Then the lookup methods would have toclonethe value and return an ownedGrmtoolsSectionValue, because there was none to borrow anymore.It feels like this solves that borrowing issue, but also is a much simpler structure, and so cleans up the code a lot?